Re: [PATCH 3/3] printk: dump full information of page flags in pGp

[Yafang Shao <laoar.shao@gmail.com>]

On Thu, Jan 28, 2021 at 8:11 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Jan 28, 2021 at 10:19:47AM +0800, Yafang Shao wrote:
> > Currently the pGp only shows the names of page flags, rather than
> > the full information including section, node, zone, last cpupid and
> > kasan tag. While it is not easy to parse these information manually
> > because there're so many flavors. Let's interpret them in pGp as well.
> >
> > - Before the patch,
> > [ 6312.639698] ERR: Slab 0x000000006d1133b9 objects=33 used=3 fp=0x000000006d0779d1 flags=0x17ffffc0010200(slab|head)
> >
> > - After the patch,
> > [ 6315.235783] ERR: Slab 0x000000006d1133b9 objects=33 used=3 fp=0x000000006d0779d1 flags=0x17ffffc0010200(Node 0x0,Zone 0x2,Lastcpupid 0x1fffff,slab|head)
>
> > +     int i;
> > +
> > +     for (i = 0; i < sizeof(pfl) / sizeof(struct page_flags_layout) && buf < end; i++) {
>
> 'buf < end' is redundant.
>

Thanks for pointing this out.

> > +             if (pfl[i].width == 0)
> > +                     continue;
> > +
> > +             buf = string(buf, end, pfl[i].name, default_str_spec);
>
> > +             if (buf >= end)
> > +                     break;
>
> Can you rather use usual patter, i.e.
>
>         if (buf < end) {
>                 ...do something...
>         }
>         buf++; // or whatever increase should be done
>
> Moreover, number() and string() IIRC have the proper checks embedded into them.
>

I will take a look at the detail in these two functions.

> > +             buf = number(buf, end, (flags >> pfl[i].shift) & pfl[i].mask,
> > +                          default_flag_spec);
>
> > +             if (buf >= end)
> > +                     break;
> > +             *buf = ',';
> > +             buf++;
>
> Here is a very standard pattern can be used, see code around
>
>                 if (buf < end)
>                         *buf = ',';
>                 buf++;
>
> > +     }
>

Thanks for the explanation.
I will change it as you suggested.


-- 
Thanks
Yafang