On Tue, Feb 21, 2023 at 03:05:26PM +0000, David Howells wrote: > Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > > > > + start = folio_pos(folio); /* May regress with THPs */ > > > > What does this comment mean? > > "start" may end up going backwards if it's pointing to the middle of a folio. So that's "regress" in the sense of "May point earlier in the file", rather than "May cause a bug" (which was how I read it)? > > > + /* At this point we hold neither the i_pages lock nor the > > > + * page lock: the page may be truncated or invalidated > > > + * (changing page->mapping to NULL), or even swizzled > > > + * back from swapper_space to tmpfs file mapping > > > > Where does this comment come from? This is cifs, not tmpfs. You'll > > never be asked to writeback a page from the swap cache. Dirty pages > > can be truncated, so the first half of the comment is still accurate. > > I'd rather it moved down to below the folio lock, and was rephrased > > so it described why we're checking everything again. > > I picked it up into afs from somewhere - nfs maybe? The same comment is in > fs/btrfs/extent_io.c. grep for 'swizzled' in fs/. You modified the comment > in b93b016313b3ba8003c3b8bb71f569af91f19fc7 in 2018, so it's been around a > while. I was just removing references to ->tree_lock ;-)