Re: [PATCH 01/10] mm: page_ref_add_unless() does not trace 'u' argument

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Dec 8, 2021 at 9:17 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:

On Wed, Dec 08, 2021 at 08:25:22PM -0500, Pasha Tatashin wrote:
On Wed, Dec 8, 2021 at 3:55 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:

On Wed, Dec 08, 2021 at 08:35:35PM +0000, Pasha Tatashin wrote:
In other page_ref_* functions all arguments and returns are traced, but
in page_ref_add_unless the 'u' argument which stands for unless boolean
is not traced. However, what is more confusing is that in the tracing
routine:
      __page_ref_mod_unless(struct page *page, int v, int u);

The 'u' argument present, but instead a return value is passed into
this argument.

Add a new template specific for page_ref_add_unless(), and trace all
arguments and the return value.

The special casing of '1' for device pages is going away, so NAK
to this user-visible change.

I can drop this patch, as it really intended to fix existing oddities
and missing info. However, I do not really understand your NAK reason.
Can you please explain about the special casing of "1" for device
pages?

$ git grep page_ref_add_unless
include/linux/mm.h:     return page_ref_add_unless(page, 1, 0);
include/linux/page_ref.h:static inline bool page_ref_add_unless(struct page *page, int nr, int u)
include/linux/page_ref.h:       return page_ref_add_unless(&folio->page, nr, u);
mm/memcontrol.c:                if (!page_ref_add_unless(page, 1, 1))

'u' is always 0, except for the caller in mm/memcontrol.c:

        if (is_device_private_entry(ent)) {
                page = pfn_swap_entry_to_page(ent);
                /*
                 * MEMORY_DEVICE_PRIVATE means ZONE_DEVICE page and which have
                 * a refcount of 1 when free (unlike normal page)
                 */
                if (!page_ref_add_unless(page, 1, 1))
                        return NULL;
                return page;
        }

That special casing of ZONE_DEVICE pages is being fixed, so 'u' will
soon always be 0, and I'm sure we'll delete it as an argument.  So
there's no point in tracing what it 'used to be'.

Sounds good, at the time when 'u' is deleted we can also address
inconsistencies fixed in this patch. So, I will drop it from my series
in the next version.

Thanks,
Pasha



[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux