Re: [PATCH v2 5/5] vsprintf: Make %pGp print the hex value

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

On Tue, Oct 19, 2021 at 10:34 PM Matthew Wilcox (Oracle)
<willy@infradead.org> wrote:
>
> All existing users of %pGp want the hex value as well as the decoded
> flag names.  This looks awkward (passing the same parameter to printf
> twice), so move that functionality into the core.  If we want, we
> can make that optional with flag arguments to %pGp in the future.
>
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

LGTM

Reviewed-by: Yafang Shao <laoar.shao@gmail.com>

> ---
>  lib/test_printf.c   | 9 +++++++--
>  lib/vsprintf.c      | 8 ++++++++
>  mm/debug.c          | 2 +-
>  mm/memory-failure.c | 8 ++++----
>  mm/page_owner.c     | 4 ++--
>  mm/slub.c           | 4 ++--
>  6 files changed, 24 insertions(+), 11 deletions(-)
>
> diff --git a/lib/test_printf.c b/lib/test_printf.c
> index d09993fca463..07309c45f327 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -609,10 +609,14 @@ page_flags_test(int section, int node, int zone, int last_cpupid,
>                 char *cmp_buf)
>  {
>         unsigned long values[] = {section, node, zone, last_cpupid, kasan_tag};
> -       unsigned long size = 0;
> +       unsigned long size;
>         bool append = false;
>         int i;
>
> +       for (i = 0; i < ARRAY_SIZE(values); i++)
> +               flags |= (values[i] & pft[i].mask) << pft[i].shift;
> +
> +       size = scnprintf(cmp_buf, BUF_SIZE, "%#lx(", flags);
>         if (flags & PAGEFLAGS_MASK) {
>                 size += scnprintf(cmp_buf + size, BUF_SIZE - size, "%s", name);
>                 append = true;
> @@ -625,7 +629,6 @@ page_flags_test(int section, int node, int zone, int last_cpupid,
>                 if (append)
>                         size += scnprintf(cmp_buf + size, BUF_SIZE - size, "|");
>
> -               flags |= (values[i] & pft[i].mask) << pft[i].shift;
>                 size += scnprintf(cmp_buf + size, BUF_SIZE - size, "%s=",
>                                 pft[i].name);
>                 size += scnprintf(cmp_buf + size, BUF_SIZE - size, pft[i].fmt,
> @@ -633,6 +636,8 @@ page_flags_test(int section, int node, int zone, int last_cpupid,
>                 append = true;
>         }
>
> +       snprintf(cmp_buf + size, BUF_SIZE - size, ")");
> +
>         test(cmp_buf, "%pGp", &flags);
>  }
>
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index d7ad44f2c8f5..214098248610 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -2023,6 +2023,11 @@ char *format_page_flags(char *buf, char *end, unsigned long flags)
>         bool append = false;
>         int i;
>
> +       buf = number(buf, end, flags, default_flag_spec);
> +       if (buf < end)
> +               *buf = '(';
> +       buf++;
> +
>         /* Page flags from the main area. */
>         if (main_flags) {
>                 buf = format_flags(buf, end, main_flags, pageflag_names);
> @@ -2051,6 +2056,9 @@ char *format_page_flags(char *buf, char *end, unsigned long flags)
>
>                 append = true;
>         }
> +       if (buf < end)
> +               *buf = ')';
> +       buf++;
>
>         return buf;
>  }
> diff --git a/mm/debug.c b/mm/debug.c
> index fae0f81ad831..714be101dec9 100644
> --- a/mm/debug.c
> +++ b/mm/debug.c
> @@ -162,7 +162,7 @@ static void __dump_page(struct page *page)
>  out_mapping:
>         BUILD_BUG_ON(ARRAY_SIZE(pageflag_names) != __NR_PAGEFLAGS + 1);
>
> -       pr_warn("%sflags: %#lx(%pGp)%s\n", type, head->flags, &head->flags,
> +       pr_warn("%sflags: %pGp%s\n", type, &head->flags,
>                 page_cma ? " CMA" : "");
>         print_hex_dump(KERN_WARNING, "raw: ", DUMP_PREFIX_NONE, 32,
>                         sizeof(unsigned long), page,
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 3e6449f2102a..e4e122a2e9b1 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -2109,14 +2109,14 @@ static int __soft_offline_page(struct page *page)
>                         if (!list_empty(&pagelist))
>                                 putback_movable_pages(&pagelist);
>
> -                       pr_info("soft offline: %#lx: %s migration failed %d, type %lx (%pGp)\n",
> -                               pfn, msg_page[huge], ret, page->flags, &page->flags);
> +                       pr_info("soft offline: %#lx: %s migration failed %d, type %pGp\n",
> +                               pfn, msg_page[huge], ret, &page->flags);
>                         if (ret > 0)
>                                 ret = -EBUSY;
>                 }
>         } else {
> -               pr_info("soft offline: %#lx: %s isolation failed, page count %d, type %lx (%pGp)\n",
> -                       pfn, msg_page[huge], page_count(page), page->flags, &page->flags);
> +               pr_info("soft offline: %#lx: %s isolation failed, page count %d, type %pGp\n",
> +                       pfn, msg_page[huge], page_count(page), &page->flags);
>                 ret = -EBUSY;
>         }
>         return ret;
> diff --git a/mm/page_owner.c b/mm/page_owner.c
> index 62402d22539b..4afc713ca525 100644
> --- a/mm/page_owner.c
> +++ b/mm/page_owner.c
> @@ -351,12 +351,12 @@ print_page_owner(char __user *buf, size_t count, unsigned long pfn,
>         pageblock_mt = get_pageblock_migratetype(page);
>         page_mt  = gfp_migratetype(page_owner->gfp_mask);
>         ret += snprintf(kbuf + ret, count - ret,
> -                       "PFN %lu type %s Block %lu type %s Flags %#lx(%pGp)\n",
> +                       "PFN %lu type %s Block %lu type %s Flags %pGp\n",
>                         pfn,
>                         migratetype_names[page_mt],
>                         pfn >> pageblock_order,
>                         migratetype_names[pageblock_mt],
> -                       page->flags, &page->flags);
> +                       &page->flags);
>
>         if (ret >= count)
>                 goto err;
> diff --git a/mm/slub.c b/mm/slub.c
> index 3d2025f7163b..f7ac28646580 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -763,9 +763,9 @@ void print_tracking(struct kmem_cache *s, void *object)
>
>  static void print_page_info(struct page *page)
>  {
> -       pr_err("Slab 0x%p objects=%u used=%u fp=0x%p flags=%#lx(%pGp)\n",
> +       pr_err("Slab 0x%p objects=%u used=%u fp=0x%p flags=%pGp\n",
>                page, page->objects, page->inuse, page->freelist,
> -              page->flags, &page->flags);
> +              &page->flags);
>
>  }
>
> --
> 2.32.0
>


-- 
Thanks
Yafang