Re: [PATCH v9 14/41] mm: swap: Introduce lru_add_drain_progressive()

[David Hildenbrand (Arm)]

On 7/31/26 16:09, David Hildenbrand (Arm) wrote:
> On 7/31/26 15:03, Ackerley Tng wrote:
>> "David Hildenbrand (Arm)" <david@xxxxxxxxxx> writes:
>>
>>>
>>> And looking at it, the current folio_ref_count() in GUP is entirely wrong
>>> and always makes us drain local+all.
>>>
>>> Let me fix that first, which will also modify this code.
>>>
>>> I'd say, to unblock gmem we could have something minimal that I would clean
>>> up later, but I think the problem is that lru_add_drain() / lru_add_drain_all()
>>> is not exported, right?
>>>
>>
>> My bad, I totally mixed up refcounting to drain with refcounting for the
>> safe conversion and didn't have a complete understanding of mlock().
>>
>> The problem for gmem was that if there was an elevated refcount on some
>> folio in the range, it might be because the folio was in the lru_add
>> fbatch.
> 
> Right.
> 
>>
>> If it were in the lru_add fbatch, it would be fine to just drain the
>> lru_add fbatch and proceed with the conversion.
>>
>> lru_add_drain() isn't sufficient since the conversion might be handled
>> on one CPU when the folio is on another CPU's fbatch, so I had to do
>> lru_add_drain_all(), but lru_add_drain_all() causes IPIs which are
>> expensive.
> 
> Yes.
> 
> You should probably do an early
> 
> folio_maybe_dma_pinned() || folio_mapped() check and just return -- don't drain.
> 
>>
>> In an earlier revision I exported lru_add_drain_all(). My understanding
>> of the discussion at guest_memfd biweekly was that Sean didn't want us
>> to export lru_add_drain_all() now, and then end up exporting some other
>> function and unexporting lru_add_drain_all(). To avoid exporting and
>> unexporting, we then said we should refactor now.
>>
>> Happy to go with your proposal too.
> Yeah, I have to look into this more closely. I'm now convinced that we really
> have to use the refcount for now, but hopefully we can limit it on
> folio_expected_refcount() internally + additional references
> from the caller.
> 
> I'm quite busy today, but let me flesh something out that actually works.
> 

Sorry for being rather grumpy previously, I know you only mean good :)


I sent a fix for the GUP code that always makes us drain right now:

https://lore.kernel.org/r/20260731-check_and_migrate_movable_folios-v1-1-e0002d7b791e@xxxxxxxxxx

Would the following (agains mm-unstable + fix) work for you?


I assume that you'd have to call it as

	lru_cache_drain_for_folio(folio, 0, NULL);

But maybe there are indeed extra references that are not reflected in
folio_expected_ref_count() that you would have to consider? I doubt it, but please check if
there would be a problem.

There is a small problem for merging:

(1) The patch depends on the fix.

(2) mm/swap.c was moved to mm/folio.c in the mm tree.

We could write what I have below against Linus' tree and have the conflict be resolved
when merging. Topic branches are unfortunately not yet a thing in MM land.