| Message ID | 20241002040111.1023018-1-willy@infradead.org |
|---|---|
| Headers | show |
| Series | Filesystem page flags cleanup | expand |
On Wed, Oct 2, 2024 at 1:02 PM Matthew Wilcox (Oracle) wrote: > > Use folio APIs instead of page APIs. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > fs/nilfs2/page.c | 22 +++++++++++----------- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/fs/nilfs2/page.c b/fs/nilfs2/page.c > index 9c0b7cddeaae..16bb82cdbc07 100644 > --- a/fs/nilfs2/page.c > +++ b/fs/nilfs2/page.c > @@ -98,16 +98,16 @@ void nilfs_forget_buffer(struct buffer_head *bh) > */ > void nilfs_copy_buffer(struct buffer_head *dbh, struct buffer_head *sbh) > { > - void *kaddr0, *kaddr1; > + void *saddr, *daddr; > unsigned long bits; > - struct page *spage = sbh->b_page, *dpage = dbh->b_page; > + struct folio *sfolio = sbh->b_folio, *dfolio = dbh->b_folio; > struct buffer_head *bh; > > - kaddr0 = kmap_local_page(spage); > - kaddr1 = kmap_local_page(dpage); > - memcpy(kaddr1 + bh_offset(dbh), kaddr0 + bh_offset(sbh), sbh->b_size); > - kunmap_local(kaddr1); > - kunmap_local(kaddr0); > + saddr = kmap_local_folio(sfolio, bh_offset(sbh)); > + daddr = kmap_local_folio(dfolio, bh_offset(dbh)); > + memcpy(daddr, saddr, sbh->b_size); > + kunmap_local(daddr); > + kunmap_local(saddr); > > dbh->b_state = sbh->b_state & NILFS_BUFFER_INHERENT_BITS; > dbh->b_blocknr = sbh->b_blocknr; > @@ -121,13 +121,13 @@ void nilfs_copy_buffer(struct buffer_head *dbh, struct buffer_head *sbh) > unlock_buffer(bh); > } > if (bits & BIT(BH_Uptodate)) > - SetPageUptodate(dpage); > + folio_mark_uptodate(dfolio); > else > - ClearPageUptodate(dpage); > + folio_clear_uptodate(dfolio); > if (bits & BIT(BH_Mapped)) > - SetPageMappedToDisk(dpage); > + folio_set_mappedtodisk(dfolio); > else > - ClearPageMappedToDisk(dpage); > + folio_clear_mappedtodisk(dfolio); > } > > /** > -- > 2.43.0 I understand the change. Also, thank you for converting this function to be folio-based. Acked-by: Ryusuke Konishi <konishi.ryusuke@gmail.com> Thanks, Ryusuke Konishi
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 Wed 02-10-24 05:01:08, Matthew Wilcox (Oracle) wrote: > These comments are now stale; rewrite them. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> Looks good. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > mm/migrate.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/mm/migrate.c b/mm/migrate.c > index df91248755e4..21264c24a404 100644 > --- a/mm/migrate.c > +++ b/mm/migrate.c > @@ -472,7 +472,7 @@ static int folio_expected_refs(struct address_space *mapping, > * The number of remaining references must be: > * 1 for anonymous folios without a mapping > * 2 for folios with a mapping > - * 3 for folios with a mapping and PagePrivate/PagePrivate2 set. > + * 3 for folios with a mapping and the private flag set. > */ > static int __folio_migrate_mapping(struct address_space *mapping, > struct folio *newfolio, struct folio *folio, int expected_count) > @@ -786,7 +786,7 @@ static int __migrate_folio(struct address_space *mapping, struct folio *dst, > * @mode: How to migrate the page. > * > * Common logic to directly migrate a single LRU folio suitable for > - * folios that do not use PagePrivate/PagePrivate2. > + * folios that do not have private data. > * > * Folios are locked upon entry and exit. > */ > -- > 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.
On Wed, 02 Oct 2024 05:01:02 +0100, Matthew Wilcox (Oracle) wrote: > I think this pile of patches makes most sense to take through the VFS > tree. The first four continue the work begun in 02e1960aafac to make the > mappedtodisk/owner_2 flag available to filesystems which don't use > buffer heads. The last two remove uses of Private2 (we're achingly > close to being rid of it entirely, but that doesn't seem like it'll > land this merge window). > > [...] Applied to the vfs.pagecache branch of the vfs/vfs.git tree. Patches in the vfs.pagecache branch should appear in linux-next soon. Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it. It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated. Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch. tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git branch: vfs.pagecache [1/6] fs: Move clearing of mappedtodisk to buffer.c https://git.kernel.org/vfs/vfs/c/9c33d85e34c2 [2/6] nilfs2: Convert nilfs_copy_buffer() to use folios https://git.kernel.org/vfs/vfs/c/a38117bc0de6 [3/6] mm: Remove PageMappedToDisk https://git.kernel.org/vfs/vfs/c/a04d5f82fa38 [4/6] btrfs: Switch from using the private_2 flag to owner_2 https://git.kernel.org/vfs/vfs/c/a6752a6e7fb0 [5/6] ceph: Remove call to PagePrivate2() https://git.kernel.org/vfs/vfs/c/fd15ba4cb00a [6/6] migrate: Remove references to Private2 https://git.kernel.org/vfs/vfs/c/7735348d9f3a