Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] i915: soft lockup in __shmem_writeback() on large sparse objects --
commit 13d0555b9d9183775f7e6a194865fdfde399b137 Author: Matthias Goergens <[email protected]> drm/i915: look up shmem folios by index for writeback This commit reverts to walking the object by index using the folio API instead of using writeback_iter(), because writeback_iter() misses shmem folios that do not set PAGECACHE_TAG_DIRTY. It introduces a manual loop in __shmem_writeback() to find folios by index. > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > index ef94401662955..54424e434f3b3 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c [ ... ] > void __shmem_writeback(size_t size, struct address_space *mapping) > { > - struct writeback_control wbc = { > - .sync_mode = WB_SYNC_NONE, > - .nr_to_write = SWAP_CLUSTER_MAX, > - .range_start = 0, > - .range_end = LLONG_MAX, > - }; > - struct folio *folio = NULL; > - int error = 0; > + pgoff_t nr_pages = size >> PAGE_SHIFT; > + pgoff_t index = 0; > > /* > + * shmem marks folios dirty with noop_dirty_folio(), which does not > + * set PAGECACHE_TAG_DIRTY, so writeback_iter() would find none of > + * them. Look up each folio of the object by index and test its > + * dirty flag. > + * > * Leave mmapings intact (GTT will have been revoked on unbinding, > * leaving only CPU mmapings around) and add those folios to the LRU > * instead of invoking writeback so they are aged and paged out > * as normal. > */ > - while ((folio = writeback_iter(mapping, &wbc, folio, &error))) { > - if (folio_mapped(folio)) > - folio_redirty_for_writepage(&wbc, folio); > + while (index < nr_pages) { > + struct folio *folio; > + > + /* > + * Skip folios that are absent or locked: this runs from the > + * shrinker and the OOM notifier, so do not wait for a lock. > + */ > + folio = __filemap_get_folio(mapping, index, > + FGP_LOCK | FGP_NOWAIT, 0); > + if (IS_ERR(folio)) { > + index++; > + continue; > + } ^^^^ [Severity: High] Can this loop cause a soft lockup for large sparse objects? If the object has a large hole, __filemap_get_folio() will return -ENOENT for missing folios. The loop then handles this error by stepping forward only 1 page at a time. For extremely large sparse objects under memory pressure, could this result in an unbounded CPU stall without yielding, blocking kswapd or direct reclaim, since there is no cond_resched() in the loop? > + > + index = folio->index; > + if (!folio_mapped(folio) && folio_clear_dirty_for_io(folio)) > + shmem_writeback_folio(folio); > else > - error = shmem_write_folio(folio); > + folio_unlock(folio); > + > + /* Writing may have split the folio, so read its size only now. > */ > + index += folio_nr_pages(folio); > + folio_put(folio); > } > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
