On Tue 2021-03-02 14:20:51, John Ogness wrote:
> On 2021-03-01, Petr Mladek <pmladek@xxxxxxxx> wrote:
> >> diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
> >> index 532f22637783..5a64b24a91c2 100644
> >> --- a/arch/powerpc/kernel/nvram_64.c
> >> +++ b/arch/powerpc/kernel/nvram_64.c
> >> @@ -681,13 +680,14 @@ static void oops_to_nvram(struct kmsg_dumper *dumper,
> >> return;
> >>
> >> if (big_oops_buf) {
> >> - kmsg_dump_get_buffer(dumper, false,
> >> + kmsg_dump_rewind(&iter);
> >
> > It would be nice to get rid of the kmsg_dump_rewind(&iter) calls
> > in all callers.
> >
> > A solution might be to create the following in include/linux/kmsg_dump.h
> >
> > Then we could do the following at the beginning of both
> > kmsg_dump_get_buffer() and kmsg_dump_get_line():
> >
> > u64 clear_seq = latched_seq_read_nolock(&clear_seq);
> >
> > if (iter->cur_seq < clear_seq)
> > cur_seq = clear_seq;
>
> I suppose we need to add this part anyway, if we want to enforce that
> records before @clear_seq are not to be available for dumpers.
Yup.
> > It might be better to avoid the infinite loop. We could do the following:
> >
> > static void check_and_set_iter(struct kmsg_dump_iter)
> > {
> > if (iter->cur_seq == 0 && iter->next_seq == U64_MAX) {
> > kmsg_dump_rewind(iter);
> > }
> >
> > and call this at the beginning of both kmsg_dump_get_buffer()
> > and kmsg_dump_get_line()
> >
> > What do you think?
>
> On a technical level, it does not make any difference. It is pure
> cosmetic.
Yup.
> Personally, I prefer the rewind directly before the kmsg_dump_get calls
> because it puts the initializer directly next to the user.
>
> As an example to illustrate my view, I prefer:
>
> for (i = 0; i < n; i++)
> ...;
>
> instead of:
>
> int i = 0;
>
> ...
>
> for (; i < n; i++)
> ...;
>
> Also, I do not really like the special use of 0/U64_MAX to identify
> special actions of the kmsg_dump_get functions.
Fair enough.
> > Note that I do not resist on it. But it might make the API easier to
> > use from my POV.
>
> Since you do not resist, I will keep the API the same for v4. But I will
> add the @clear_seq check to the kmsg_dump_get functions.
Go for it.
Best Regards,
Petr
