Hello,

[email protected], le lun. 13 oct. 2025 21:13:01 +0200, a ecrit:
> Another thing for the streamio change is that my current patch does not work 
> when there is a both an input and an output buffer.
> 
> Can this happen?

Yes, see the open_hook which allows both modes at the same time.

> If yes do you have an idea on how to resolve this?

What issue do you actually get?  From a quick glance it doesn't look
like calling start_input/output spuriously would really pose problem?

Samuel

> Sep 28, 2025, 19:32 by [email protected]:
> 
> > [email protected], le dim. 28 sept. 2025 20:37:01 +0200, a ecrit:
> >
> >>
> >>
> >>
> >> Sep 25, 2025, 21:02 by [email protected]:
> >>
> >> > Hello,
> >> >
> >> > [email protected], le jeu. 25 sept. 2025 13:11:07 +0200, a ecrit:
> >> >
> >> >> Hi SamuelI found something in kmsg_putchar.
> >> >>
> >> >> if kmsg_write_offset +1 == kmsg_read_offset (mod KMSGBUFFSIZE) 
> >> >> it just discards the character which leads to the previous message 
> >> >> being malformed.
> >> >>
> >> >
> >> > Which is kind of expected: we are overflowing.
> >> >
> >> >> Should it in that case also terminate the current message by setting 
> >> >> the previously written char (at kmsg_write_offset -1  mod 
> >> >> KMSGBUFFERSIZE) to \n ?
> >> >>
> >> >
> >> > I'd rather not alter the content.
> >> >
> >> >> I tried this patch and it fixes the problem. Some messages are being 
> >> >> lost but I don't know what else to do with them. (i am reusing the 
> >> >> offset variable that is not needed anymore)
> >> >> I guess I encounter this because at boot time a lot of messages are 
> >> >> being written to the buffer in a short time before the syslogd can 
> >> >> start emptying them so increasing the buffer prevents the early 
> >> >> wraparound.
> >> >>
> >> >
> >> > Yes, there is no way around that issue except increasing the buffer size
> >> > if our boot log is indeed quite verbose (we can indeed do that). Perhaps
> >> > we'd rather instead drop some messages which are not really that useful.
> >> >
> >>
> >> I am overflowing by just a couple of messages, with the increased buffer I 
> >> can read 4438 bytes.
> >> > That being said, dropping the latest characters being put might not be
> >> > the best, I'd rather say drop the oldest characters so that you are sure
> >> > you have the latest information. And then you'll have '\n'.
> >> >
> >> This essentially means writing the char as usual but when after that write 
> >> and read offset are equal incrementing the read offset as well.
> >>
> >> Something like this?
> >>
> >
> > Yes, exactly.
> >
> > Samuel
> >
> >> --8<---------------cut here---------------start------------->8---
> >> diff --git a/device/kmsg.c b/device/kmsg.c
> >> index e5b518e6..bb72930d 100644
> >> --- a/device/kmsg.c
> >> +++ b/device/kmsg.c
> >> @@ -217,7 +217,6 @@ void
> >> kmsg_putchar (int c)
> >> {
> >>    io_req_t ior;
> >> -  int offset;
> >>    spl_t s = -1;
> >>  
> >>    /* XXX: cninit is not called before cnputc is used. So call kmsginit
> >> @@ -230,22 +229,20 @@ kmsg_putchar (int c)
> >>  
> >>    if (spl_init)
> >>      s = simple_lock_irq (&kmsg_lock);
> >> -  offset = kmsg_write_offset + 1;
> >> -  if (offset == KMSGBUFSIZE)
> >> -    offset = 0;
> >> -
> >> -  if (offset == kmsg_read_offset)
> >> -    {
> >> -      /* Discard C.  */
> >> -      if (spl_init)
> >> -  simple_unlock_irq (s, &kmsg_lock);
> >> -      return;
> >> -    }
> >>  
> >>    kmsg_buffer[kmsg_write_offset++] = c;
> >>    if (kmsg_write_offset == KMSGBUFSIZE)
> >>      kmsg_write_offset = 0;
> >>  
> >> +  if(kmsg_write_offset == kmsg_read_offset)
> >> +    {
> >> +      /* Drop first unread char */
> >> +      kmsg_read_offset++;
> >> +      if (kmsg_read_offset == KMSGBUFSIZE)
> >> +        kmsg_read_offset = 0;
> >> +    }
> >> +
> >> +
> >>    while ((ior = (io_req_t) dequeue_head (&kmsg_read_queue)) != NULL)
> >>      iodone (ior);
> >> --8<---------------cut here---------------end--------------->8---
> >>
> >> I think I would also prefer this over just dropping the character.
> >>
> >>
> >> > Samuel
> >> >
> >>
> >
> > -- 
> > Samuel
> > /*
> >  * Oops. The kernel tried to access some bad page. We'll have to
> >  * terminate things with extreme prejudice.
> > */
> > die_if_kernel("Oops", regs, error_code);
> > (From linux/arch/i386/mm/fault.c)
> >
> 

-- 
Samuel
--- christ gives channel operator status to Dieu
 -+- #ens-mim and hell -+-

Reply via email to