Please, please, please send a cover letter when there's > 1 patch :) This 2/2 replying to 1/2 is a pain (not your fault that perhaps you're not aware of typical mm series style but FYI :P)
Also there is some tiny conflict on khugepaged.c in mm-new, but it's literally 1 #include so probably nothing to worry about. On Mon, Nov 10, 2025 at 11:32:55AM +0000, Shivank Garg wrote: > When MADV_COLLAPSE encounters dirty file-backed pages, it currently > returns -EINVAL, this is misleading as EINVAL suggests invalid arguments, > whereas dirty pages are a transient condition that may resolve on retry. > > Introduce SCAN_PAGE_DIRTY and map it to -EAGAIN. For khugepaged, this > is harmless as it will revisit the range after async writeback completes. > > Signed-off-by: Shivank Garg <[email protected]> With comments below addressed, LGTM so: Reviewed-by: Lorenzo Stoakes <[email protected]> > --- > include/trace/events/huge_memory.h | 3 ++- > mm/khugepaged.c | 4 +++- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/include/trace/events/huge_memory.h > b/include/trace/events/huge_memory.h > index dd94d14a2427..9014a9bbe64c 100644 > --- a/include/trace/events/huge_memory.h > +++ b/include/trace/events/huge_memory.h > @@ -38,7 +38,8 @@ > EM( SCAN_PAGE_HAS_PRIVATE, "page_has_private") \ > EM( SCAN_STORE_FAILED, "store_failed") \ > EM( SCAN_COPY_MC, "copy_poisoned_page") \ > - EMe(SCAN_PAGE_FILLED, "page_filled") > + EM(SCAN_PAGE_FILLED, "page_filled") \ > + EMe(SCAN_PAGE_DIRTY, "page_dirty") > > #undef EM > #undef EMe > diff --git a/mm/khugepaged.c b/mm/khugepaged.c > index d08ed6eb9ce1..7df329c9c87d 100644 > --- a/mm/khugepaged.c > +++ b/mm/khugepaged.c > @@ -60,6 +60,7 @@ enum scan_result { > SCAN_STORE_FAILED, > SCAN_COPY_MC, > SCAN_PAGE_FILLED, > + SCAN_PAGE_DIRTY, it feels like a lot to add a scan result for this, but I mean... probably actually valid. > }; > > #define CREATE_TRACE_POINTS > @@ -1967,7 +1968,7 @@ static int collapse_file(struct mm_struct *mm, unsigned > long addr, > */ > xas_unlock_irq(&xas); > filemap_flush(mapping); > - result = SCAN_FAIL; > + result = SCAN_PAGE_DIRTY; > goto xa_unlocked; Hmmm shmem dirty is going to be weird but we also have: if (!is_shmem && (folio_test_dirty(folio) || folio_test_writeback(folio))) { /* * khugepaged only works on read-only fd, so this * folio is dirty because it hasn't been flushed * since first write. */ result = SCAN_FAIL; goto out_unlock; } It's weird though, why would we have writeback, surely handled by swap, and won't it be like anon, i.e. pretty well always dirty? This comment seems copy/pasta wrong. We do need to at least mention in commit message that shmem is explicitly excluded. > } else if (folio_test_writeback(folio)) { > xas_unlock_irq(&xas); > @@ -2747,6 +2748,7 @@ static int madvise_collapse_errno(enum scan_result r) > case SCAN_PAGE_LRU: > case SCAN_DEL_PAGE_LRU: > case SCAN_PAGE_FILLED: > + case SCAN_PAGE_DIRTY: > return -EAGAIN; > /* > * Other: Trying again likely not to succeed / error intrinsic to > -- > 2.43.0 >
