| Message ID | 20211012182647.1605095-3-willy@infradead.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Improvements to %pGp | expand |
On Wed, Oct 13, 2021 at 2:31 AM Matthew Wilcox (Oracle) <willy@infradead.org> wrote: > > Even unknown flags should be passed to %pGp so that we can test what it > does with them (today: nothing). > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> Reviewed-by: Yafang Shao <laoar.shao@gmail.com> > --- > lib/test_printf.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/lib/test_printf.c b/lib/test_printf.c > index a52c1c3a55ba..f744b0498672 100644 > --- a/lib/test_printf.c > +++ b/lib/test_printf.c > @@ -608,14 +608,12 @@ page_flags_test(int section, int node, int zone, int last_cpupid, > int kasan_tag, int flags, const char *name, char *cmp_buf) > { > unsigned long values[] = {section, node, zone, last_cpupid, kasan_tag}; > - unsigned long page_flags = 0; > + unsigned long page_flags = flags; > unsigned long size = 0; > bool append = false; > int i; > > - flags &= PAGEFLAGS_MASK; > - if (flags) { > - page_flags |= flags; > + if (flags & PAGEFLAGS_MASK) { > snprintf(cmp_buf + size, BUF_SIZE - size, "%s", name); > size = strlen(cmp_buf); > #if SECTIONS_WIDTH || NODES_WIDTH || ZONES_WIDTH || \ > -- > 2.32.0 >
On Tue, Oct 12, 2021 at 07:26:44PM +0100, Matthew Wilcox (Oracle) wrote: > Even unknown flags should be passed to %pGp so that we can test what it > does with them (today: nothing). > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > lib/test_printf.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/lib/test_printf.c b/lib/test_printf.c > index a52c1c3a55ba..f744b0498672 100644 > --- a/lib/test_printf.c > +++ b/lib/test_printf.c > @@ -608,14 +608,12 @@ page_flags_test(int section, int node, int zone, int last_cpupid, > int kasan_tag, int flags, const char *name, char *cmp_buf) > { > unsigned long values[] = {section, node, zone, last_cpupid, kasan_tag}; > - unsigned long page_flags = 0; > + unsigned long page_flags = flags; > unsigned long size = 0; > bool append = false; > int i; > > - flags &= PAGEFLAGS_MASK; > - if (flags) { > - page_flags |= flags; > + if (flags & PAGEFLAGS_MASK) { > snprintf(cmp_buf + size, BUF_SIZE - size, "%s", name); > size = strlen(cmp_buf); > #if SECTIONS_WIDTH || NODES_WIDTH || ZONES_WIDTH || \ Do we even need 'page_flags'? Can't we do everything on 'flags' directly?
On Tue 2021-10-12 19:26:44, Matthew Wilcox (Oracle) wrote: > Even unknown flags should be passed to %pGp so that we can test what it > does with them (today: nothing). > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> Makes sense. Reviewed-by: Petr Mladek <pmladek@suse.com> Best Regards, Petr
On Wed 2021-10-13 12:25:30, Kirill A. Shutemov wrote: > On Tue, Oct 12, 2021 at 07:26:44PM +0100, Matthew Wilcox (Oracle) wrote: > > Even unknown flags should be passed to %pGp so that we can test what it > > does with them (today: nothing). > > > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > > --- > > lib/test_printf.c | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/lib/test_printf.c b/lib/test_printf.c > > index a52c1c3a55ba..f744b0498672 100644 > > --- a/lib/test_printf.c > > +++ b/lib/test_printf.c > > @@ -608,14 +608,12 @@ page_flags_test(int section, int node, int zone, int last_cpupid, > > int kasan_tag, int flags, const char *name, char *cmp_buf) > > { > > unsigned long values[] = {section, node, zone, last_cpupid, kasan_tag}; > > - unsigned long page_flags = 0; > > + unsigned long page_flags = flags; > > unsigned long size = 0; > > bool append = false; > > int i; > > > > - flags &= PAGEFLAGS_MASK; > > - if (flags) { > > - page_flags |= flags; > > + if (flags & PAGEFLAGS_MASK) { > > snprintf(cmp_buf + size, BUF_SIZE - size, "%s", name); > > size = strlen(cmp_buf); > > #if SECTIONS_WIDTH || NODES_WIDTH || ZONES_WIDTH || \ > > Do we even need 'page_flags'? Can't we do everything on 'flags' directly? I have to say that the meaning of "flags" and "page_flags" variable is far from clear [*] It would be worth documentation how they are supposed to be used. I am going to comment more on this in the 5th patch. Best Regards, Petr [*] Shame on me. I did not pay much attention to the test code when reviewing the original code.
On 10/13/21 2:55 PM, Kirill A. Shutemov wrote: > On Tue, Oct 12, 2021 at 07:26:44PM +0100, Matthew Wilcox (Oracle) wrote: >> Even unknown flags should be passed to %pGp so that we can test what it >> does with them (today: nothing). >> >> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> >> --- >> lib/test_printf.c | 6 ++---- >> 1 file changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/lib/test_printf.c b/lib/test_printf.c >> index a52c1c3a55ba..f744b0498672 100644 >> --- a/lib/test_printf.c >> +++ b/lib/test_printf.c >> @@ -608,14 +608,12 @@ page_flags_test(int section, int node, int zone, int last_cpupid, >> int kasan_tag, int flags, const char *name, char *cmp_buf) >> { >> unsigned long values[] = {section, node, zone, last_cpupid, kasan_tag}; >> - unsigned long page_flags = 0; >> + unsigned long page_flags = flags; >> unsigned long size = 0; >> bool append = false; >> int i; >> >> - flags &= PAGEFLAGS_MASK; >> - if (flags) { >> - page_flags |= flags; >> + if (flags & PAGEFLAGS_MASK) { >> snprintf(cmp_buf + size, BUF_SIZE - size, "%s", name); >> size = strlen(cmp_buf); >> #if SECTIONS_WIDTH || NODES_WIDTH || ZONES_WIDTH || \ > > Do we even need 'page_flags'? Can't we do everything on 'flags' directly? Agreed, +1. Otherwise the change LGTM.
diff --git a/lib/test_printf.c b/lib/test_printf.c index a52c1c3a55ba..f744b0498672 100644 --- a/lib/test_printf.c +++ b/lib/test_printf.c @@ -608,14 +608,12 @@ page_flags_test(int section, int node, int zone, int last_cpupid, int kasan_tag, int flags, const char *name, char *cmp_buf) { unsigned long values[] = {section, node, zone, last_cpupid, kasan_tag}; - unsigned long page_flags = 0; + unsigned long page_flags = flags; unsigned long size = 0; bool append = false; int i; - flags &= PAGEFLAGS_MASK; - if (flags) { - page_flags |= flags; + if (flags & PAGEFLAGS_MASK) { snprintf(cmp_buf + size, BUF_SIZE - size, "%s", name); size = strlen(cmp_buf); #if SECTIONS_WIDTH || NODES_WIDTH || ZONES_WIDTH || \
Even unknown flags should be passed to %pGp so that we can test what it does with them (today: nothing). Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- lib/test_printf.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)