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@xxxxxxxxxxxxxxxxxxxx/ (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.
