[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <64284E33-3828-46E9-AFFB-649E0DA41023@vmware.com>
Date: Mon, 9 Dec 2019 08:54:55 +0000
From: Ajay Kaher <akaher@...are.com>
To: Vlastimil Babka <vbabka@...e.cz>,
"stable@...r.kernel.org" <stable@...r.kernel.org>
CC: "linux-mm@...ck.org" <linux-mm@...ck.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Jann Horn <jannh@...gle.com>,
Matthew Wilcox <willy@...radead.org>,
"stable@...nel.org" <stable@...nel.org>,
Srivatsa Bhat <srivatsab@...are.com>,
"srivatsa@...il.mit.edu" <srivatsa@...il.mit.edu>,
Vasavi Sirnapalli <vsirnapalli@...are.com>,
"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
Subject: Re: [PATCH STABLE 4.4 5/8] mm: prevent get_user_pages() from
overflowing page refcount
On 06/12/19, 8:02 PM, "Vlastimil Babka" <vbabka@...e.cz> wrote:
> On 12/6/19 5:15 AM, Ajay Kaher wrote:
>>
>>
>> On 03/12/19, 6:28 PM, "Vlastimil Babka" <vbabka@...e.cz> wrote:
>>>>>
>>>>> [ 4.4 backport: there's get_page_foll(), so add try_get_page()-like checks
>>>>> in there, enabled by a new parameter, which is false where
>>>>> upstream patch doesn't replace get_page() with try_get_page()
>>>>> (the THP and hugetlb callers).
>>>>
>>>> Could we have try_get_page_foll(), as in:
>>>> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fstable%2F1570581863-12090-3-git-send-email-akaher%40vmware.com%2F&data=02%7C01%7Cakaher%40vmware.com%7Cb65cf5622ca8401fd2ba08d77a5914e8%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637112395344338606&sdata=sLbw%2BQWu0%2BB0y2OpfaQS%2FxXX6Z9jNB3wPeTcPsawNJA%3D&reserved=0
>>>>
>>>> + Code will be in sync as we have try_get_page()
>>>> + No need to add extra argument to try_get_page()
>>>> + No need to modify the callers of try_get_page()
>>
>> Any reason for not using try_get_page_foll().
>
> Ah, sorry, I missed that previously. It's certainly possible to do it
> that way, I just didn't care so strongly to rewrite the existing SLES
> patch. It's a stable backport for a rather old LTS, not a codebase for
> further development.
Thanks for your response.
I would appreciate if you would like to include try_get_page_foll(),
and resend this patch series again.
Greg may require Acked-by from my side also, so if it's fine with you,
you can add or I will add once you will post this patch series again.
Let me know if anything else I can do here.
>>>>> In gup_pte_range(), we don't expect tail pages, so just check
>>>>> page ref count instead of try_get_compound_head()
>>>>
>>>> Technically it's fine. If you want to keep the code of stable versions in sync
>>>> with latest versions then this could be done in following ways (without any
>>>> modification in upstream patch for gup_pte_range()):
>>>>
>>>> Apply 7aef4172c7957d7e65fc172be4c99becaef855d4 before applying
>>>> 8fde12ca79aff9b5ba951fce1a2641901b8d8e64, as done here:
>>>> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fstable%2F1570581863-12090-4-git-send-email-akaher%40vmware.com%2F&data=02%7C01%7Cakaher%40vmware.com%7Cb65cf5622ca8401fd2ba08d77a5914e8%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637112395344348599&sdata=MYA%2Fx7oVu8x1c7%2FGkEw%2B69FX7WN1O34Oq8lkMiFs1Wk%3D&reserved=0
>>
>>> Yup, I have considered that, and deliberately didn't add that commit
>>> 7aef4172c795 ("mm: handle PTE-mapped tail pages in gerneric fast gup
>>> implementaiton") as it's part of a large THP refcount rework. In 4.4 we
>>> don't expect to GUP tail pages so I wanted to keep it that way -
>>> minimally, the compound_head() operation is a unnecessary added cost,
>>> although it would also work.
>>
Thanks for above explanation.
Powered by blists - more mailing lists