Messages in this thread |  | | Date | Thu, 11 Feb 2021 12:05:51 +0100 | From | Petr Mladek <> | Subject | Re: [PATCH] printk: avoid prb_first_valid_seq() where possible |
| |
On Wed 2021-02-10 19:32:10, John Ogness wrote: > On 2021-02-09, Petr Mladek <pmladek@suse.com> wrote: > >> @@ -1629,9 +1631,13 @@ int do_syslog(int type, char __user *buf, int len, int source) > >> /* Number of chars in the log buffer */ > >> case SYSLOG_ACTION_SIZE_UNREAD: > >> logbuf_lock_irq(); > >> - if (syslog_seq < prb_first_valid_seq(prb)) { > >> - /* messages are gone, move to first one */ > >> - syslog_seq = prb_first_valid_seq(prb); > >> + if (prb_read_valid_info(prb, syslog_seq, &info, NULL)) { > >> + if (info.seq != syslog_seq) { > >> + /* messages are gone, move to first one */ > >> + syslog_seq = info.seq; > >> + syslog_partial = 0; > >> + } > >> + } else { > >> syslog_partial = 0; > > > > I am scratching my head when prb_read_valid_info(prb, > > syslog_seq, &info, NULL)) might fail. > > It can fail because the descriptor has been invalidated/recycled by > writers and perhaps there is no valid record that has yet come after it.
I see. From some reasons I though that there should always be at least one message in the commited state. But it is enough when it is in reusable state. I should have double checked it.
> I recommend changing your suggestion to: > > > if (!prb_read_valid_info(prb, syslog_seq, &info, NULL)) { > > /* > > * No unread messages. No need to check/reset > > * syslog_partial. When a reader does read a new > > * message it will notice and appropriately update > > * syslog_seq and reset syslog_partial. > > */
The following comment might be enough after all.
/* No unread messages. */
My main concern was that we cleared syslog_partial and continued. I thought that we might miss a bug this way. But it seems to be perfectly fine. I just have to update my mental picture.
Otherwise. the fact that syslog_partial will be fixed by the next successful call is more or less obvious if we change the code as you propose.
Please, send an updated patch.
Best Regards, Petr
> > logbuf_unlock_irq(); > > return 0; > > } > > if (info.seq != syslog_seq) { > > /* messages are gone, move to first one */ > > syslog_seq = info.seq; > > syslog_partial = 0; > > } > > John Ogness
|  |