On 2/4/21 11:27 AM, Joao Martins wrote:
> On 2/3/21 11:00 PM, John Hubbard wrote:
>> On 2/3/21 2:00 PM, Joao Martins wrote:
>>> Add an helper that iterates over head pages in a list of pages. It
>>> essentially counts the tails until the next page to process has a
>>> different head that the current. This is going to be used by
>>> unpin_user_pages() family of functions, to batch the head page refcount
>>> updates once for all passed consecutive tail pages.
>>>
>>> Suggested-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
>>> Signed-off-by: Joao Martins <joao.m.martins@xxxxxxxxxx>
>>> ---
>>> mm/gup.c | 29 +++++++++++++++++++++++++++++
>>> 1 file changed, 29 insertions(+)
>>>
>>> diff --git a/mm/gup.c b/mm/gup.c
>>> index d68bcb482b11..4f88dcef39f2 100644
>>> --- a/mm/gup.c
>>> +++ b/mm/gup.c
>>> @@ -215,6 +215,35 @@ void unpin_user_page(struct page *page)
>>> }
>>> EXPORT_SYMBOL(unpin_user_page);
>>>
>>> +static inline unsigned int count_ntails(struct page **pages, unsigned long npages)
>>
>> Silly naming nit: could we please name this function count_pagetails()? count_ntails
>> is a bit redundant, plus slightly less clear.
>>
> Hmm, pagetails is also a tiny bit redundant. Perhaps count_subpages() instead?
>
> count_ntails is meant to be 'count number of tails' i.e. to align terminology with head +
> tails which was also suggested over the other series.
>
Given your comment on the third patch, I reworked a bit and got rid of the count_ntails.
So it's looking like this, also the macro arguments renaming as well):
diff --git a/mm/gup.c b/mm/gup.c
index d68bcb482b11..d1549c61c2f6 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -215,6 +215,35 @@ void unpin_user_page(struct page *page)
}
EXPORT_SYMBOL(unpin_user_page);
+static inline void compound_next(unsigned long i, unsigned long npages,
+ struct page **list, struct page **head,
+ unsigned int *ntails)
+{
+ struct page *page;
+ unsigned int nr;
+
+ if (i >= npages)
+ return;
+
+ list += i;
+ npages -= i;
+ page = compound_head(*list);
+
+ for (nr = 1; nr < npages; nr++) {
+ if (compound_head(list[nr]) != page)
+ break;
+ }
+
+ *head = page;
+ *ntails = nr;
+}
+
+#define for_each_compound_head(__i, __list, __npages, __head, __ntails) \
+ for (__i = 0, \
+ compound_next(__i, __npages, __list, &(__head), &(__ntails)); \
+ __i < __npages; __i += __ntails, \
+ compound_next(__i, __npages, __list, &(__head), &(__ntails)))
+
/**
* unpin_user_pages_dirty_lock() - release and optionally dirty gup-pinned pages
* @pages: array of pages to be maybe marked dirty, and definitely released.
