1. Patchew
  2. linux
  3. [PATCH v2 0/6] mm: better GUP pin lru_add_drain_all()
  4. View patch

[PATCH v2 2/6] mm/gup: local lru_add_drain() to avoid lru_add_drain_all()

Hugh Dickins posted 6 patches 1 day, 3 hours ago
[PATCH v2 2/6] mm/gup: local lru_add_drain() to avoid lru_add_drain_all()
Posted by Hugh Dickins 1 day, 3 hours ago 
In many cases, if collect_longterm_unpinnable_folios() does need to
drain the LRU cache to release a reference, the cache in question is
on this same CPU, and much more efficiently drained by a preliminary
local lru_add_drain(), than the later cross-CPU lru_add_drain_all().

Marked for stable, to counter the increase in lru_add_drain_all()s
from "mm/gup: check ref_count instead of lru before migration".
Note for clean backports: can take 6.16 commit a03db236aebf ("gup:
optimize longterm pin_user_pages() for large folio") first.

Signed-off-by: Hugh Dickins <hughd@google.com>
Cc: <stable@vger.kernel.org>
---
 mm/gup.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/mm/gup.c b/mm/gup.c
index 82aec6443c0a..b47066a54f52 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2287,8 +2287,8 @@ static unsigned long collect_longterm_unpinnable_folios(
 		struct pages_or_folios *pofs)
 {
 	unsigned long collected = 0;
-	bool drain_allow = true;
 	struct folio *folio;
+	int drained = 0;
 	long i = 0;
 
 	for (folio = pofs_get_folio(pofs, i); folio;
@@ -2307,10 +2307,17 @@ static unsigned long collect_longterm_unpinnable_folios(
 			continue;
 		}
 
-		if (drain_allow && folio_ref_count(folio) !=
-				   folio_expected_ref_count(folio) + 1) {
+		if (drained == 0 &&
+				folio_ref_count(folio) !=
+				folio_expected_ref_count(folio) + 1) {
+			lru_add_drain();
+			drained = 1;
+		}
+		if (drained == 1 &&
+				folio_ref_count(folio) !=
+				folio_expected_ref_count(folio) + 1) {
 			lru_add_drain_all();
-			drain_allow = false;
+			drained = 2;
 		}
 
 		if (!folio_isolate_lru(folio))
-- 
2.51.0
Re: [PATCH v2 2/6] mm/gup: local lru_add_drain() to avoid lru_add_drain_all()
Posted by David Hildenbrand 18 hours ago 
On 09.09.25 00:16, Hugh Dickins wrote:
> In many cases, if collect_longterm_unpinnable_folios() does need to
> drain the LRU cache to release a reference, the cache in question is
> on this same CPU, and much more efficiently drained by a preliminary
> local lru_add_drain(), than the later cross-CPU lru_add_drain_all().
> 
> Marked for stable, to counter the increase in lru_add_drain_all()s
> from "mm/gup: check ref_count instead of lru before migration".
> Note for clean backports: can take 6.16 commit a03db236aebf ("gup:
> optimize longterm pin_user_pages() for large folio") first.
> 
> Signed-off-by: Hugh Dickins <hughd@google.com>
> Cc: <stable@vger.kernel.org>
> ---
>   mm/gup.c | 15 +++++++++++----
>   1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/gup.c b/mm/gup.c
> index 82aec6443c0a..b47066a54f52 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -2287,8 +2287,8 @@ static unsigned long collect_longterm_unpinnable_folios(
>   		struct pages_or_folios *pofs)
>   {
>   	unsigned long collected = 0;
> -	bool drain_allow = true;
>   	struct folio *folio;
> +	int drained = 0;
>   	long i = 0;
>   
>   	for (folio = pofs_get_folio(pofs, i); folio;
> @@ -2307,10 +2307,17 @@ static unsigned long collect_longterm_unpinnable_folios(
>   			continue;
>   		}
>   
> -		if (drain_allow && folio_ref_count(folio) !=
> -				   folio_expected_ref_count(folio) + 1) {
> +		if (drained == 0 &&
> +				folio_ref_count(folio) !=
> +				folio_expected_ref_count(folio) + 1) {

I would just have indented this as follows:

		if (drained == 0 &&
		    folio_ref_count(folio) != folio_expected_ref_count(folio) + 1) {

Same below.

In any case logic LGTM

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers

David / dhildenb
Re: [PATCH v2 2/6] mm/gup: local lru_add_drain() to avoid lru_add_drain_all()
Posted by Kiryl Shutsemau 15 hours ago 
On Tue, Sep 09, 2025 at 09:56:30AM +0200, David Hildenbrand wrote:
> On 09.09.25 00:16, Hugh Dickins wrote:
> > In many cases, if collect_longterm_unpinnable_folios() does need to
> > drain the LRU cache to release a reference, the cache in question is
> > on this same CPU, and much more efficiently drained by a preliminary
> > local lru_add_drain(), than the later cross-CPU lru_add_drain_all().
> > 
> > Marked for stable, to counter the increase in lru_add_drain_all()s
> > from "mm/gup: check ref_count instead of lru before migration".
> > Note for clean backports: can take 6.16 commit a03db236aebf ("gup:
> > optimize longterm pin_user_pages() for large folio") first.
> > 
> > Signed-off-by: Hugh Dickins <hughd@google.com>
> > Cc: <stable@vger.kernel.org>
> > ---
> >   mm/gup.c | 15 +++++++++++----
> >   1 file changed, 11 insertions(+), 4 deletions(-)
> > 
> > diff --git a/mm/gup.c b/mm/gup.c
> > index 82aec6443c0a..b47066a54f52 100644
> > --- a/mm/gup.c
> > +++ b/mm/gup.c
> > @@ -2287,8 +2287,8 @@ static unsigned long collect_longterm_unpinnable_folios(
> >   		struct pages_or_folios *pofs)
> >   {
> >   	unsigned long collected = 0;
> > -	bool drain_allow = true;
> >   	struct folio *folio;
> > +	int drained = 0;
> >   	long i = 0;
> >   	for (folio = pofs_get_folio(pofs, i); folio;
> > @@ -2307,10 +2307,17 @@ static unsigned long collect_longterm_unpinnable_folios(
> >   			continue;
> >   		}
> > -		if (drain_allow && folio_ref_count(folio) !=
> > -				   folio_expected_ref_count(folio) + 1) {
> > +		if (drained == 0 &&
> > +				folio_ref_count(folio) !=
> > +				folio_expected_ref_count(folio) + 1) {
> 
> I would just have indented this as follows:
> 
> 		if (drained == 0 &&
> 		    folio_ref_count(folio) != folio_expected_ref_count(folio) + 1) {

Do we want folio_check_expected_ref_count(folio, offset)?

-- 
  Kiryl Shutsemau / Kirill A. Shutemov
Re: [PATCH v2 2/6] mm/gup: local lru_add_drain() to avoid lru_add_drain_all()
Posted by David Hildenbrand 14 hours ago 
On 09.09.25 12:52, Kiryl Shutsemau wrote:
> On Tue, Sep 09, 2025 at 09:56:30AM +0200, David Hildenbrand wrote:
>> On 09.09.25 00:16, Hugh Dickins wrote:
>>> In many cases, if collect_longterm_unpinnable_folios() does need to
>>> drain the LRU cache to release a reference, the cache in question is
>>> on this same CPU, and much more efficiently drained by a preliminary
>>> local lru_add_drain(), than the later cross-CPU lru_add_drain_all().
>>>
>>> Marked for stable, to counter the increase in lru_add_drain_all()s
>>> from "mm/gup: check ref_count instead of lru before migration".
>>> Note for clean backports: can take 6.16 commit a03db236aebf ("gup:
>>> optimize longterm pin_user_pages() for large folio") first.
>>>
>>> Signed-off-by: Hugh Dickins <hughd@google.com>
>>> Cc: <stable@vger.kernel.org>
>>> ---
>>>    mm/gup.c | 15 +++++++++++----
>>>    1 file changed, 11 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/mm/gup.c b/mm/gup.c
>>> index 82aec6443c0a..b47066a54f52 100644
>>> --- a/mm/gup.c
>>> +++ b/mm/gup.c
>>> @@ -2287,8 +2287,8 @@ static unsigned long collect_longterm_unpinnable_folios(
>>>    		struct pages_or_folios *pofs)
>>>    {
>>>    	unsigned long collected = 0;
>>> -	bool drain_allow = true;
>>>    	struct folio *folio;
>>> +	int drained = 0;
>>>    	long i = 0;
>>>    	for (folio = pofs_get_folio(pofs, i); folio;
>>> @@ -2307,10 +2307,17 @@ static unsigned long collect_longterm_unpinnable_folios(
>>>    			continue;
>>>    		}
>>> -		if (drain_allow && folio_ref_count(folio) !=
>>> -				   folio_expected_ref_count(folio) + 1) {
>>> +		if (drained == 0 &&
>>> +				folio_ref_count(folio) !=
>>> +				folio_expected_ref_count(folio) + 1) {
>>
>> I would just have indented this as follows:
>>
>> 		if (drained == 0 &&
>> 		    folio_ref_count(folio) != folio_expected_ref_count(folio) + 1) {
> 
> Do we want folio_check_expected_ref_count(folio, offset)?

Not sure, if so outside of this patch series to also cover the other 
handful of cases.

	folio_has_unexpected_refs(folio, offset)

Would probably be clearer.

-- 
Cheers

David / dhildenb

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.