On 06/12/19, 8:02 PM, "Vlastimil Babka" vbabka@suse.cz wrote:
On 12/6/19 5:15 AM, Ajay Kaher wrote:
On 03/12/19, 6:28 PM, "Vlastimil Babka" vbabka@suse.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.kerne...
- 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.kerne...
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.