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

[Petr Mladek]

On Tue 2021-02-09 16:16:01, Andy Shevchenko wrote:
> On Tue, Feb 09, 2021 at 02:53:53PM +0100, Petr Mladek wrote:
> > On Tue 2021-02-09 18:56:13, Yafang Shao wrote:
> 
> ...
> 
> > I am sorry for my ignorance. I am not familiar with MM.
> > But it is pretty hard to understand what call does what.
> > 
> > I have found the following comment in include/linux/page_flags.h:
> > 
> >  * The page flags field is split into two parts, the main flags area
> >  * which extends from the low bits upwards, and the fields area which
> >  * extends from the high bits downwards.
> > 
> > Sigh, I know that you already reworked this several times because
> > people "nitpicked" about the code style. But it seems that it
> > rather diverged instead of converged.
> > 
> > What about the following?
> 
> Isn't is some like v1 or v2?

Yes. And people suggested only some micro-optimizations and reported
few small bugs there.

But the code was heavily reworked in v3 to support the new
%pGp[bl] variants. It also added the trick with the bitmap
which invalidated all the previous suggestions.

v3 and v4 review focused on behavior changes. We finally
agreed on it. Let's give it more cycle and clean up the code
after so many shuffles.


> > Note: It is inpired by the names "main area" and "fields area"
> >       mentioned in the above comment from page_flags.h.
> >       I have later realized that "page_flags_layout" actually made
> >       sense as well. Feel free to rename page_flags_fileds
> >       back to page_flags_layout.
> > 
> > Anyway, this is my proposal:
> 
> What about to create a one format_flags() function which accepts new data
> structure and do something like
> 
> buf = format_flags(main_area);
> buf = format_flags(fields_area);
> return buf;

I am not sure that it would make things easier. The handling of
the main area is trivial and reuses existing structures. The handling
of the fields area seems to be much more complicated.

Best Regards,
Petr