[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3db5a5e8-62f3-4915-9fae-49a7f88952c9@redhat.com>
Date: Tue, 14 Jan 2025 18:23:12 +0100
From: David Hildenbrand <david@...hat.com>
To: Yang Shi <yang@...amperecomputing.com>,
Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Cc: arnd@...db.de, gregkh@...uxfoundation.org, Liam.Howlett@...cle.com,
vbabka@...e.cz, jannh@...gle.com, willy@...radead.org,
liushixin2@...wei.com, akpm@...ux-foundation.org, linux-mm@...ck.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] /dev/zero: make private mapping full anonymous mapping
On 14.01.25 18:01, Yang Shi wrote:
>
>
>
> On 1/14/25 7:06 AM, David Hildenbrand wrote:
>> On 14.01.25 15:52, Lorenzo Stoakes wrote:
>>> On Tue, Jan 14, 2025 at 02:01:32PM +0100, David Hildenbrand wrote:
>>>> On 13.01.25 23:30, Yang Shi wrote:
>>>>> When creating private mapping for /dev/zero, the driver makes it an
>>>>> anonymous mapping by calling set_vma_anonymous(). But it just sets
>>>>> vm_ops to NULL, vm_file is still valid and vm_pgoff is also file
>>>>> offset.
>>>>>
>>>>> This is a special case and the VMA doesn't look like either
>>>>> anonymous VMA
>>>>> or file VMA. It confused other kernel subsystem, for example,
>>>>> khugepaged [1].
>>>>>
>>>>> It seems pointless to keep such special case. Making private
>>>>> /dev/zero>
>>>> mapping a full anonymous mapping doesn't change the semantic of
>>>>> /dev/zero either.
>>>>>
>>>>> The user visible effect is the mapping entry shown in
>>>>> /proc/<PID>/smaps
>>>>> and /proc/<PID>/maps.
>>>>>
>>>>> Before the change:
>>>>> ffffb7190000-ffffb7590000 rw-p 00001000 00:06
>>>>> 8 /dev/zero
>>>>>
>>>>> After the change:
>>>>> ffffb6130000-ffffb6530000 rw-p 00000000 00:00 0
>>>>>
>>>>
>>>> Hm, not sure about this. It's actually quite consistent to have that
>>>> output
>>>> in smaps the way it is. You mapped a file at an offset, and it
>>>> behaves like
>>>> an anonymous mapping apart from that.
>>>>
>>>> Not sure if the buggy khugepaged thing is a good indicator to
>>>> warrant this
>>>> change.
>
> I admit this may be a concern, but I doubt who really care about it...
>
There is an example in the man page [1] about /proc/self/map_files/.
I assume that will also change here.
It's always hard to tell who that could affect, but I'm not convinced
this is worth it to find it out :)
>>>
>>> Yeah, this is a user-facing fundamental change that hides information
>>> and
>>> defies expectation so I mean - it's a no go really isn't it?
>>>
>>> I'd rather we _not_ make this anon though, because isn't life confusing
>>> enough David? I thought it was bad enough with 'anon, file and lol
>>> shmem'
>>> but 'lol lol also /dev/zero' is enough to make me want to frolick in the
>>> fields...
>>
>> I recall there are users that rely on this memory to get the shared
>> zeropage on reads etc (in comparison to shmem!), so I better not ...
>> mess with this *at all* :)
>
> The behavior won't be changed.
Yes, I know. And that's good ;)
[1] https://man7.org/linux/man-pages/man5/proc_pid_map_files.5.html
--
Cheers,
David / dhildenb
Powered by blists - more mailing lists