[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9a420e97-cc3d-da6b-5955-e6c31d96164c@redhat.com>
Date: Fri, 2 Oct 2020 16:57:03 +0200
From: David Hildenbrand <david@...hat.com>
To: Matthew Wilcox <willy@...radead.org>
Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org,
linux-hyperv@...r.kernel.org, xen-devel@...ts.xenproject.org,
linux-acpi@...r.kernel.org,
Andrew Morton <akpm@...ux-foundation.org>,
Alexander Duyck <alexander.h.duyck@...ux.intel.com>,
Vlastimil Babka <vbabka@...e.cz>,
Oscar Salvador <osalvador@...e.de>,
Mel Gorman <mgorman@...hsingularity.net>,
Michal Hocko <mhocko@...nel.org>,
Dave Hansen <dave.hansen@...el.com>,
Wei Yang <richard.weiyang@...ux.alibaba.com>,
Mike Rapoport <rppt@...nel.org>
Subject: Re: [PATCH v1 1/5] mm/page_alloc: convert "report" flag of
__free_one_page() to a proper flag
On 02.10.20 16:48, David Hildenbrand wrote:
> On 02.10.20 15:41, Matthew Wilcox wrote:
>> On Mon, Sep 28, 2020 at 08:21:06PM +0200, David Hildenbrand wrote:
>>> Let's prepare for additional flags and avoid long parameter lists of bools.
>>> Follow-up patches will also make use of the flags in __free_pages_ok(),
>>> however, I wasn't able to come up with a better name for the type - should
>>> be good enough for internal purposes.
>>
>>> +/* Free One Page flags: for internal, non-pcp variants of free_pages(). */
>>> +typedef int __bitwise fop_t;
>>
>> That invites confusion with f_op. There's no reason to use _t as a suffix
>> here ... why not free_f?
>
> git grep "bitwise" | grep typedef | grep include/linux
>
> indicates that "_t" it the right thing to do.
>
> I want a name that highlights that is is for the internal variants of
> free_page(), free_f / free_t is too generic.
>
> fpi_t (Free Page Internal) ?
>
>>
>>> +/*
>>> + * Skip free page reporting notification for the (possibly merged) page. (will
>>> + * *not* mark the page reported, only skip the notification).
>>
>> ... Don't you mean "will not skip marking the page as reported, only
>> skip the notification"?
>
> Yeah, I can use that.
Reading again, it doesn't quite fit. Marking pages as reported is
handled by mm/page_reporting.c
/*
* Skip free page reporting notification for the (possibly merged) page.
* This does not hinder free page reporting from grabbing the page,
* reporting it and marking it "reported" - it only skips notifying
* the free page reporting infrastructure about a newly freed page. For
* example, used when temporarily pulling a page from the freelist and
* putting it back unmodified.
*/
Is that clearer?
--
Thanks,
David / dhildenb
Powered by blists - more mailing lists