| Message ID | 20241002040111.1023018-2-willy@infradead.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Filesystem page flags cleanup | expand |
On Wed 02-10-24 05:01:03, Matthew Wilcox (Oracle) wrote: > The mappedtodisk flag is only meaningful for buffer head based > filesystems. It should not be cleared for other filesystems. This allows > us to reuse the mappedtodisk flag to have other meanings in filesystems > that do not use buffer heads. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> The patch looks good. But I'm bit confused about the changelog. There's no generic code checking for mappedtodisk. Only nilfs2 actually uses it for anything, all other filesystems just never look at it as far as my grepping shows. So speaking about "filesystems that do not use buffer heads" looks somewhat broad to me. Anyway feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > fs/buffer.c | 1 + > mm/truncate.c | 1 - > 2 files changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/buffer.c b/fs/buffer.c > index 1fc9a50def0b..35f9af799e0a 100644 > --- a/fs/buffer.c > +++ b/fs/buffer.c > @@ -1649,6 +1649,7 @@ void block_invalidate_folio(struct folio *folio, size_t offset, size_t length) > if (length == folio_size(folio)) > filemap_release_folio(folio, 0); > out: > + folio_clear_mappedtodisk(folio); > return; > } > EXPORT_SYMBOL(block_invalidate_folio); > diff --git a/mm/truncate.c b/mm/truncate.c > index 0668cd340a46..870af79fb446 100644 > --- a/mm/truncate.c > +++ b/mm/truncate.c > @@ -166,7 +166,6 @@ static void truncate_cleanup_folio(struct folio *folio) > * Hence dirty accounting check is placed after invalidation. > */ > folio_cancel_dirty(folio); > - folio_clear_mappedtodisk(folio); > } > > int truncate_inode_folio(struct address_space *mapping, struct folio *folio) > -- > 2.43.0 > >
On Thu, Oct 03, 2024 at 02:10:20PM +0200, Jan Kara wrote: > On Wed 02-10-24 05:01:03, Matthew Wilcox (Oracle) wrote: > > The mappedtodisk flag is only meaningful for buffer head based > > filesystems. It should not be cleared for other filesystems. This allows > > us to reuse the mappedtodisk flag to have other meanings in filesystems > > that do not use buffer heads. > > > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > > The patch looks good. But I'm bit confused about the changelog. There's no > generic code checking for mappedtodisk. Only nilfs2 actually uses it for > anything, all other filesystems just never look at it as far as my grepping > shows. So speaking about "filesystems that do not use buffer heads" looks > somewhat broad to me. Anyway feel free to add: Hmm. f2fs also uses it in page_mkwrite(). But it looks odd to me. Perhaps we could get rid of mappedtodisk entirely ... I see ext4 used to use it until someone removed it in 9ea7df534ed2 ;-) Anyway, what the changelog is trying to say is that only buffer-head filesystems ever have the mappedtodisk flag set, eg by block_read_full_folio() or do_mpage_readpage(). So it doesn't make sense to clear it for non-buffer-head filesystems, and may inhibit their ability to use it for unrelated purposes.
diff --git a/fs/buffer.c b/fs/buffer.c index 1fc9a50def0b..35f9af799e0a 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -1649,6 +1649,7 @@ void block_invalidate_folio(struct folio *folio, size_t offset, size_t length) if (length == folio_size(folio)) filemap_release_folio(folio, 0); out: + folio_clear_mappedtodisk(folio); return; } EXPORT_SYMBOL(block_invalidate_folio); diff --git a/mm/truncate.c b/mm/truncate.c index 0668cd340a46..870af79fb446 100644 --- a/mm/truncate.c +++ b/mm/truncate.c @@ -166,7 +166,6 @@ static void truncate_cleanup_folio(struct folio *folio) * Hence dirty accounting check is placed after invalidation. */ folio_cancel_dirty(folio); - folio_clear_mappedtodisk(folio); } int truncate_inode_folio(struct address_space *mapping, struct folio *folio)
The mappedtodisk flag is only meaningful for buffer head based filesystems. It should not be cleared for other filesystems. This allows us to reuse the mappedtodisk flag to have other meanings in filesystems that do not use buffer heads. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/buffer.c | 1 + mm/truncate.c | 1 - 2 files changed, 1 insertion(+), 1 deletion(-)