Re: Removing ->dirty_folio
John Hubbard <[email protected]>
| Newsgroups | org.kvack.linux-mm,org.kernel.vger.linux-btrfs,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-xfs |
|---|---|
| Message-ID | <[email protected]> |
On 8/24/26 12:43 PM, Matthew Wilcox wrote: > On Mon, Aug 24, 2026 at 12:25:42PM -0700, John Hubbard wrote: >>> My proposal is this: >>> >>> - Fileystems take note of folio_maybe_dma_pinned() during writeback. >>> If it's true, do the writeback, but retain/recreate whatever data >>> structures you need in order to write the folio again; behave as if >>> ->page_mkdirty() had been called again for each page in the folio is >>> marked as dirty. >> >> Yes, that would work nicely. >> >>> - The MM behaves similarly; we do not clear the writeback flag for >>> folio_maybe_dma_pinned(). >>> >>> This will have the effect of writing pinned folios back every time the >>> inode is scheduled for writeback. But since we have no idea whether >>> the folio is actually dirty (because the GUP user won't tell us), >>> this is the correct behaviour. >>> >>> I'm probably missing some stuff here. Let me know. >> >> OK, so working through the end of the pinning, I think it still is >> correct: device finishes writing to pinned memory, device driver >> unpins the memory but the page has been left marked dirty the whole oh, I just thought of a minor hole that we need to fill: how to mark the page dirty in the first place, in the absence of mark_[page|folio]_dirty()? Under this new scheme, we will need to pin first, then mark dirty, to set up. The filesystem can't do everything, because even if it were to call page_mkdirty(), a writeback could clear that before the page gets pinned. Then teardown is simply to unpin, as discussed already. >> time and still is, so the next writeback still does the writeback, >> but this time sees no pins and so it marks the page clean. > > Excellent! By the way, what would you think to this? > > @@ -2717,7 +2719,8 @@ static inline bool folio_maybe_dma_pinned(struct folio *folio) > * Here, for that overflow case, use the sign bit to count a little > * bit higher via unsigned math, and thus still get an accurate result. > */ > - return ((unsigned int)folio_ref_count(folio)) >= > + mapcount = folio_mapcount(folio); > + return (folio_ref_count(folio) - mapcount) >= As long as the math works: need to not underflow. I guess mapcount is always less than refcount, so OK. So it *seems* correct to me, fwiw. :) > GUP_PIN_COUNTING_BIAS; > } > > > It should improve the accuracy of folio_maybe_dma_pinned() for folios > which are mapped many, many times (eg a page of libc). I'm a little > concerned about races turning that number negative since we don't > necessarily have the folio locked at that point. thanks, -- John Hubbard