[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8fb9915b-73a1-4e39-bb30-c1c21b1b7650@gmail.com>
Date: Fri, 16 Aug 2024 17:41:29 +0100
From: Usama Arif <usamaarif642@...il.com>
To: Matthew Wilcox <willy@...radead.org>
Cc: akpm@...ux-foundation.org, linux-mm@...ck.org, hannes@...xchg.org,
riel@...riel.com, shakeel.butt@...ux.dev, roman.gushchin@...ux.dev,
yuzhao@...gle.com, david@...hat.com, baohua@...nel.org,
ryan.roberts@....com, rppt@...nel.org, cerasuolodomenico@...il.com,
corbet@....net, linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
kernel-team@...a.com
Subject: Re: [PATCH v3 4/6] mm: Introduce a pageflag for partially mapped
folios
On 16/08/2024 17:28, Matthew Wilcox wrote:
> On Fri, Aug 16, 2024 at 05:08:35PM +0100, Usama Arif wrote:
>> On 16/08/2024 16:44, Matthew Wilcox wrote:
>>> On Tue, Aug 13, 2024 at 01:02:47PM +0100, Usama Arif wrote:
>>>> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
>>>> index a0a29bd092f8..cecc1bad7910 100644
>>>> --- a/include/linux/page-flags.h
>>>> +++ b/include/linux/page-flags.h
>>>> @@ -182,6 +182,7 @@ enum pageflags {
>>>> /* At least one page in this folio has the hwpoison flag set */
>>>> PG_has_hwpoisoned = PG_active,
>>>> PG_large_rmappable = PG_workingset, /* anon or file-backed */
>>>> + PG_partially_mapped, /* was identified to be partially mapped */
>>>
>>> No, you can't do this. You have to be really careful when reusing page
>>> flags, you can't just take the next one. What made you think it would
>>> be this easy?
>>>
>>> I'd suggest using PG_reclaim. You also need to add PG_partially_mapped
>>> to PAGE_FLAGS_SECOND. You might get away without that if you're
>>> guaranteeing it'll always be clear when you free the folio; I don't
>>> understand this series so I don't know if that's true or not.
>>
>> I am really not sure what the issue is over here.
>
> You've made the code more fragile. It might happen to work today, but
> you've either done something which is subtly broken today, or might
> break tomorrow when somebody else rearranges the flags without knowing
> about your fragility.
>
>> >From what I see, bits 0-7 of folio->_flags_1 are used for storing folio order, bit 8 for PG_has_hwpoisoned and bit 9 for PG_large_rmappable.
>> Bits 10 and above of folio->_flags_1 are not used any anywhere in the kernel. I am not reusing a page flag of folio->_flags_1, just taking an unused one.
>
> No, wrong. PG_anon_exclusive is used on every page, including tail
> pages, and that's above bit 10.
>
>> Please have a look at the next few lines of the patch. I have defined the functions as FOLIO_FLAG(partially_mapped, FOLIO_SECOND_PAGE). I believe thats what you are saying in your second paragraph?
>> I am not sure what you meant by using PG_reclaim.
>
> I mean:
>
> - PG_usama_new_thing,
> + PG_usama_new_thing = PG_reclaim,
>
Ah ok, Thanks. The flags below PG_reclaim are either marked as PF_ANY or are arch dependent. So eventhough they might not be used currently for _flags_1, they could be in the future.
I will use PG_partially_mapped = PG_reclaim in the next revision.
Thanks
Powered by blists - more mailing lists