Re: [PATCH v2] mm/migrate: put dest folio on deferred split list if source was there.

[Matthew Wilcox <willy@infradead.org>]

On Tue, Mar 12, 2024 at 10:13:16AM -0400, Zi Yan wrote:
> On 11 Mar 2024, at 23:45, Matthew Wilcox wrote:
> > Much more important: You're doing this with a positive refcount, which
> > breaks the (undocumented) logic in deferred_split_scan() that a folio
> > with a positive refcount will not be removed from the list.
> 
> What is the issue here? I thought as long as the split_queue_lock is held,
> it should be OK to manipulate the list.

I just worked this out yesterday:
https://lore.kernel.org/linux-mm/Ze9EFdFLXQEUVtKl@casper.infradead.org/
(the last chunk, starting with Ryan asking me "what about the first bug
you found")

> > Maximally important: Wer shouldn't be doing any of this!  This folio is
> > on the deferred split list.  We shouldn't be migrating it as a single
> > entity; we should be splitting it now that we're in a context where we
> > can do the right thing and split it.  Documentation/mm/transhuge.rst
> > is clear that we don't split it straight away due to locking context.
> > Splitting it on migration is clearly the right thing to do.
> >
> > If splitting fails, we should just fail the migration; splitting fails
> > due to excess references, and if the source folio has excess references,
> > then migration would fail too.
> 
> You are suggesting:
> 1. checking if the folio is on deferred split list or not
> 2. if yes, split the folio
> 3. if split fails, fail the migration as well.
> 
> It sounds reasonable to me. The split folios should be migrated since
> the before-split folio wants to be migrated. This split is not because
> no new page cannot be allocated, thus the split folios should go
> into ret_folios list instead of split_folios list.

Yes, I'm happy for the split folios to be migrated.  Bonus points if you
want to figure out what order to split the folio to ;-)  I don't think
it's critical.