[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <38D49672-674B-43F8-AFD3-73BFD8876DCE@linux.dev>
Date: Fri, 28 Jun 2024 10:35:16 +0800
From: Muchun Song <muchun.song@...ux.dev>
To: Yu Zhao <yuzhao@...gle.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
David Hildenbrand <david@...hat.com>,
Frank van der Linden <fvdl@...gle.com>,
"Matthew Wilcox (Oracle)" <willy@...radead.org>,
Peter Xu <peterx@...hat.com>,
Yang Shi <yang@...amperecomputing.com>,
Linux-MM <linux-mm@...ck.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH mm-unstable v2] mm/hugetlb_vmemmap: fix race with
speculative PFN walkers
> On Jun 28, 2024, at 07:04, Yu Zhao <yuzhao@...gle.com> wrote:
>
> On Thu, Jun 27, 2024 at 4:47 PM Andrew Morton <akpm@...ux-foundation.org> wrote:
>>
>> On Thu, 27 Jun 2024 16:27:05 -0600 Yu Zhao <yuzhao@...gle.com> wrote:
>>
>>> While investigating HVO for THPs [1], it turns out that speculative
>>> PFN walkers like compaction can race with vmemmap modifications, e.g.,
>>>
>>> CPU 1 (vmemmap modifier) CPU 2 (speculative PFN walker)
>>> ------------------------------- ------------------------------
>>> Allocates an LRU folio page1
>>> Sees page1
>>> Frees page1
>>>
>>> Allocates a hugeTLB folio page2
>>> (page1 being a tail of page2)
>>>
>>> Updates vmemmap mapping page1
>>> get_page_unless_zero(page1)
>>>
>>> Even though page1->_refcount is zero after HVO, get_page_unless_zero()
>>> can still try to modify this read-only field, resulting in a crash.
>>
>> Ah. So we should backport this into earlier kernels, yes?
>>
>> Are we able to identify a Fixes: for this? Looks difficult.
>>
>> This seems quite hard to trigger. Do any particular userspace actions
>> invoke the race?
>
> Yes, *very* hard to trigger:
> 1. Most hugeTLB use cases I know of are static, i.e., reserved at boot
> time, because allocating at runtime is not reliable at all.
> 2. On top of that, someone has to be very unlucky to get tripped over
> above, because the race window is so small -- I wasn't able to trigger
> it with a stress testing that does nothing but that (with THPs
> though).
>
> So I don't think it's worth cc'ing stable, unless Muchun recommends.
I agree with Yu.
Thanks.
Powered by blists - more mailing lists