* [PATCH 0/2] Avoid setting folio->private @ 2025-02-21 20:20 Goldwyn Rodrigues 2025-02-21 20:20 ` [PATCH 1/2] btrfs: add mapping_set_release_always to inode's mapping Goldwyn Rodrigues 2025-02-21 20:20 ` [PATCH 2/2] btrfs: kill EXTENT_FOLIO_PRIVATE Goldwyn Rodrigues 0 siblings, 2 replies; 6+ messages in thread From: Goldwyn Rodrigues @ 2025-02-21 20:20 UTC (permalink / raw) To: linux-btrfs; +Cc: Goldwyn Rodrigues From: Goldwyn Rodrigues <rgoldwyn@suse.com> folio->private was set in order to get release_folio() callbacks. Instead if we set address_space flags AS_RELEASE_ALWAYS, there is no need to set EXTENT_FOLIO_PRIVATE on every folio->private. These patches are posted so we don't face conflicts with iomap's iomap_folio_state, which also resides in folio->private. Goldwyn Rodrigues (2): btrfs: add mapping_set_release_always to inode's mapping btrfs: kill EXTENT_FOLIO_PRIVATE fs/btrfs/compression.c | 2 +- fs/btrfs/defrag.c | 2 +- fs/btrfs/extent_io.c | 30 ++++++++++++++---------------- fs/btrfs/extent_io.h | 10 ++-------- fs/btrfs/file.c | 6 +++--- fs/btrfs/free-space-cache.c | 9 --------- fs/btrfs/inode.c | 9 ++++++--- fs/btrfs/reflink.c | 2 +- fs/btrfs/relocation.c | 2 +- 9 files changed, 29 insertions(+), 43 deletions(-) -- 2.48.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] btrfs: add mapping_set_release_always to inode's mapping 2025-02-21 20:20 [PATCH 0/2] Avoid setting folio->private Goldwyn Rodrigues @ 2025-02-21 20:20 ` Goldwyn Rodrigues 2025-02-28 14:23 ` David Sterba 2025-02-21 20:20 ` [PATCH 2/2] btrfs: kill EXTENT_FOLIO_PRIVATE Goldwyn Rodrigues 1 sibling, 1 reply; 6+ messages in thread From: Goldwyn Rodrigues @ 2025-02-21 20:20 UTC (permalink / raw) To: linux-btrfs; +Cc: Goldwyn Rodrigues From: Goldwyn Rodrigues <rgoldwyn@suse.com> Set mapping_set_release_always() on inode's address_space during inode initialization so release_folio() is always called. Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com> --- fs/btrfs/extent_io.c | 2 +- fs/btrfs/file.c | 2 +- fs/btrfs/inode.c | 3 +++ 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index 7b0aa332aedc..d1f9fad18f25 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -491,7 +491,7 @@ static void begin_folio_read(struct btrfs_fs_info *fs_info, struct folio *folio) if (!btrfs_is_subpage(fs_info, folio->mapping)) return; - ASSERT(folio_test_private(folio)); + ASSERT(mapping_release_always(folio->mapping)); btrfs_folio_set_lock(fs_info, folio, folio_pos(folio), PAGE_SIZE); } diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index 2bf57f0e92d2..5808eb5bcd42 100644 --- a/fs/btrfs/file.c +++ b/fs/btrfs/file.c @@ -832,7 +832,7 @@ static int prepare_uptodate_folio(struct inode *inode, struct folio *folio, u64 * The private flag check is essential for subpage as we need to store * extra bitmap using folio private. */ - if (folio->mapping != inode->i_mapping || !folio_test_private(folio)) { + if (folio->mapping != inode->i_mapping || !mapping_release_always(folio->mapping)) { folio_unlock(folio); return -EAGAIN; } diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index ace9a3ecdefa..6424d45c6baa 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -5602,6 +5602,8 @@ static int btrfs_init_locked_inode(struct inode *inode, void *p) btrfs_set_inode_number(BTRFS_I(inode), args->ino); BTRFS_I(inode)->root = btrfs_grab_root(args->root); + mapping_set_release_always(inode->i_mapping); + if (args->root && args->root == args->root->fs_info->tree_root && args->ino != BTRFS_BTREE_INODE_OBJECTID) set_bit(BTRFS_INODE_FREE_SPACE_INODE, @@ -6673,6 +6675,7 @@ static int btrfs_create(struct mnt_idmap *idmap, struct inode *dir, inode->i_fop = &btrfs_file_operations; inode->i_op = &btrfs_file_inode_operations; inode->i_mapping->a_ops = &btrfs_aops; + mapping_set_release_always(inode->i_mapping); return btrfs_create_common(dir, dentry, inode); } -- 2.48.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] btrfs: add mapping_set_release_always to inode's mapping 2025-02-21 20:20 ` [PATCH 1/2] btrfs: add mapping_set_release_always to inode's mapping Goldwyn Rodrigues @ 2025-02-28 14:23 ` David Sterba 0 siblings, 0 replies; 6+ messages in thread From: David Sterba @ 2025-02-28 14:23 UTC (permalink / raw) To: Goldwyn Rodrigues; +Cc: linux-btrfs, Goldwyn Rodrigues On Fri, Feb 21, 2025 at 03:20:52PM -0500, Goldwyn Rodrigues wrote: > From: Goldwyn Rodrigues <rgoldwyn@suse.com> > > Set mapping_set_release_always() on inode's address_space during > inode initialization so release_folio() is always called. This is a bit terse for an explanation but is correct. I had to look up what it does, basically adds a flag so that folio_needs_release() is always true. > > Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com> > --- > fs/btrfs/extent_io.c | 2 +- > fs/btrfs/file.c | 2 +- > fs/btrfs/inode.c | 3 +++ > 3 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index 7b0aa332aedc..d1f9fad18f25 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -491,7 +491,7 @@ static void begin_folio_read(struct btrfs_fs_info *fs_info, struct folio *folio) > if (!btrfs_is_subpage(fs_info, folio->mapping)) > return; > > - ASSERT(folio_test_private(folio)); > + ASSERT(mapping_release_always(folio->mapping)); > btrfs_folio_set_lock(fs_info, folio, folio_pos(folio), PAGE_SIZE); > } > > diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c > index 2bf57f0e92d2..5808eb5bcd42 100644 > --- a/fs/btrfs/file.c > +++ b/fs/btrfs/file.c > @@ -832,7 +832,7 @@ static int prepare_uptodate_folio(struct inode *inode, struct folio *folio, u64 > * The private flag check is essential for subpage as we need to store > * extra bitmap using folio private. The comment mentions private but but you're removing it so it needs an update too. > */ > - if (folio->mapping != inode->i_mapping || !folio_test_private(folio)) { > + if (folio->mapping != inode->i_mapping || !mapping_release_always(folio->mapping)) { > folio_unlock(folio); > return -EAGAIN; > } > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index ace9a3ecdefa..6424d45c6baa 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -5602,6 +5602,8 @@ static int btrfs_init_locked_inode(struct inode *inode, void *p) > btrfs_set_inode_number(BTRFS_I(inode), args->ino); > BTRFS_I(inode)->root = btrfs_grab_root(args->root); > > + mapping_set_release_always(inode->i_mapping); > + > if (args->root && args->root == args->root->fs_info->tree_root && > args->ino != BTRFS_BTREE_INODE_OBJECTID) > set_bit(BTRFS_INODE_FREE_SPACE_INODE, > @@ -6673,6 +6675,7 @@ static int btrfs_create(struct mnt_idmap *idmap, struct inode *dir, > inode->i_fop = &btrfs_file_operations; > inode->i_op = &btrfs_file_inode_operations; > inode->i_mapping->a_ops = &btrfs_aops; > + mapping_set_release_always(inode->i_mapping); > return btrfs_create_common(dir, dentry, inode); > } > > -- > 2.48.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] btrfs: kill EXTENT_FOLIO_PRIVATE 2025-02-21 20:20 [PATCH 0/2] Avoid setting folio->private Goldwyn Rodrigues 2025-02-21 20:20 ` [PATCH 1/2] btrfs: add mapping_set_release_always to inode's mapping Goldwyn Rodrigues @ 2025-02-21 20:20 ` Goldwyn Rodrigues 2025-02-28 14:37 ` David Sterba 1 sibling, 1 reply; 6+ messages in thread From: Goldwyn Rodrigues @ 2025-02-21 20:20 UTC (permalink / raw) To: linux-btrfs; +Cc: Goldwyn Rodrigues From: Goldwyn Rodrigues <rgoldwyn@suse.com> Since mapping_set_release_always() will provide a callback for release folio, remove EXTENT_FOLIO_PRIVATE and all it's helper functions. This affects how we handle subpage, so convert the function name of set_folio_extent_mapped() to btrfs_set_folio_subpage() and clear_folio_extent_mapped() to btrfs_clear_folio_subpage(). free_space_cache_inode does not use subpage, so just skip calling btrfs_set_folio_subpage(). Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com> --- fs/btrfs/compression.c | 2 +- fs/btrfs/defrag.c | 2 +- fs/btrfs/extent_io.c | 28 +++++++++++++--------------- fs/btrfs/extent_io.h | 10 ++-------- fs/btrfs/file.c | 4 ++-- fs/btrfs/free-space-cache.c | 9 --------- fs/btrfs/inode.c | 6 +++--- fs/btrfs/reflink.c | 2 +- fs/btrfs/relocation.c | 2 +- 9 files changed, 24 insertions(+), 41 deletions(-) diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c index 1fe154e7cc02..8f0cc726ba20 100644 --- a/fs/btrfs/compression.c +++ b/fs/btrfs/compression.c @@ -491,7 +491,7 @@ static noinline int add_ra_bio_pages(struct inode *inode, *memstall = 1; } - ret = set_folio_extent_mapped(folio); + ret = btrfs_set_folio_subpage(folio); if (ret < 0) { folio_unlock(folio); folio_put(folio); diff --git a/fs/btrfs/defrag.c b/fs/btrfs/defrag.c index 968dae953948..46df90f9e790 100644 --- a/fs/btrfs/defrag.c +++ b/fs/btrfs/defrag.c @@ -883,7 +883,7 @@ static struct folio *defrag_prepare_one_folio(struct btrfs_inode *inode, pgoff_t return ERR_PTR(-ETXTBSY); } - ret = set_folio_extent_mapped(folio); + ret = btrfs_set_folio_subpage(folio); if (ret < 0) { folio_unlock(folio); folio_put(folio); diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index d1f9fad18f25..ca29a1111de6 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -865,25 +865,22 @@ static int attach_extent_buffer_folio(struct extent_buffer *eb, return ret; } -int set_folio_extent_mapped(struct folio *folio) +int btrfs_set_folio_subpage(struct folio *folio) { - struct btrfs_fs_info *fs_info; + struct btrfs_fs_info *fs_info = folio_to_fs_info(folio); ASSERT(folio->mapping); - if (folio_test_private(folio)) + if (!btrfs_is_subpage(fs_info, folio->mapping)) return 0; - fs_info = folio_to_fs_info(folio); - - if (btrfs_is_subpage(fs_info, folio->mapping)) - return btrfs_attach_subpage(fs_info, folio, BTRFS_SUBPAGE_DATA); + if (folio_test_private(folio)) + return 0; - folio_attach_private(folio, (void *)EXTENT_FOLIO_PRIVATE); - return 0; + return btrfs_attach_subpage(fs_info, folio, BTRFS_SUBPAGE_DATA); } -void clear_folio_extent_mapped(struct folio *folio) +void btrfs_clear_folio_subpage(struct folio *folio) { struct btrfs_fs_info *fs_info; @@ -893,10 +890,11 @@ void clear_folio_extent_mapped(struct folio *folio) return; fs_info = folio_to_fs_info(folio); - if (btrfs_is_subpage(fs_info, folio->mapping)) - return btrfs_detach_subpage(fs_info, folio, BTRFS_SUBPAGE_DATA); - folio_detach_private(folio); + if (!btrfs_is_subpage(fs_info, folio->mapping)) + return; + + btrfs_detach_subpage(fs_info, folio, BTRFS_SUBPAGE_DATA); } static struct extent_map *get_extent_map(struct btrfs_inode *inode, @@ -951,7 +949,7 @@ static int btrfs_do_readpage(struct folio *folio, struct extent_map **em_cached, size_t iosize; size_t blocksize = fs_info->sectorsize; - ret = set_folio_extent_mapped(folio); + ret = btrfs_set_folio_subpage(folio); if (ret < 0) { folio_unlock(folio); return ret; @@ -1562,7 +1560,7 @@ static int extent_writepage(struct folio *folio, struct btrfs_bio_ctrl *bio_ctrl * The proper bitmap can only be initialized until writepage_delalloc(). */ bio_ctrl->submit_bitmap = (unsigned long)-1; - ret = set_folio_extent_mapped(folio); + ret = btrfs_set_folio_subpage(folio); if (ret < 0) goto done; diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h index 6c5328bfabc2..303c92272a46 100644 --- a/fs/btrfs/extent_io.h +++ b/fs/btrfs/extent_io.h @@ -65,12 +65,6 @@ enum { ENUM_BIT(PAGE_SET_ORDERED), }; -/* - * Folio private values. Every page that is controlled by the extent map has - * folio private set to this value. - */ -#define EXTENT_FOLIO_PRIVATE 1 - /* * The extent buffer bitmap operations are done with byte granularity instead of * word granularity for two reasons: @@ -247,8 +241,8 @@ int btrfs_writepages(struct address_space *mapping, struct writeback_control *wb int btree_write_cache_pages(struct address_space *mapping, struct writeback_control *wbc); void btrfs_readahead(struct readahead_control *rac); -int set_folio_extent_mapped(struct folio *folio); -void clear_folio_extent_mapped(struct folio *folio); +int btrfs_set_folio_subpage(struct folio *folio); +void btrfs_clear_folio_subpage(struct folio *folio); struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info, u64 start, u64 owner_root, int level); diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index 5808eb5bcd42..820feaf26583 100644 --- a/fs/btrfs/file.c +++ b/fs/btrfs/file.c @@ -875,7 +875,7 @@ static noinline int prepare_one_folio(struct inode *inode, struct folio **folio_ } /* Only support page sized folio yet. */ ASSERT(folio_order(folio) == 0); - ret = set_folio_extent_mapped(folio); + ret = btrfs_set_folio_subpage(folio); if (ret < 0) { folio_unlock(folio); folio_put(folio); @@ -1840,7 +1840,7 @@ static vm_fault_t btrfs_page_mkwrite(struct vm_fault *vmf) folio_wait_writeback(folio); lock_extent(io_tree, page_start, page_end, &cached_state); - ret2 = set_folio_extent_mapped(folio); + ret2 = btrfs_set_folio_subpage(folio); if (ret2 < 0) { ret = vmf_error(ret2); unlock_extent(io_tree, page_start, page_end, &cached_state); diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c index 056546bf9abd..7a85b243f18e 100644 --- a/fs/btrfs/free-space-cache.c +++ b/fs/btrfs/free-space-cache.c @@ -453,7 +453,6 @@ static int io_ctl_prepare_pages(struct btrfs_io_ctl *io_ctl, bool uptodate) int i; for (i = 0; i < io_ctl->num_pages; i++) { - int ret; folio = __filemap_get_folio(inode->i_mapping, i, FGP_LOCK | FGP_ACCESSED | FGP_CREAT, @@ -463,14 +462,6 @@ static int io_ctl_prepare_pages(struct btrfs_io_ctl *io_ctl, bool uptodate) return -ENOMEM; } - ret = set_folio_extent_mapped(folio); - if (ret < 0) { - folio_unlock(folio); - folio_put(folio); - io_ctl_drop_pages(io_ctl); - return ret; - } - io_ctl->pages[i] = &folio->page; if (uptodate && !folio_test_uptodate(folio)) { btrfs_read_folio(NULL, folio); diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 6424d45c6baa..896c9454dc96 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -4863,7 +4863,7 @@ int btrfs_truncate_block(struct btrfs_inode *inode, loff_t from, loff_t len, * folio private, but left the page in the mapping. Set the page mapped * here to make sure it's properly set for the subpage stuff. */ - ret = set_folio_extent_mapped(folio); + ret = btrfs_set_folio_subpage(folio); if (ret < 0) goto out_unlock; @@ -7290,7 +7290,7 @@ static bool __btrfs_release_folio(struct folio *folio, gfp_t gfp_flags) { if (try_release_extent_mapping(folio, gfp_flags)) { wait_subpage_spinlock(folio); - clear_folio_extent_mapped(folio); + btrfs_clear_folio_subpage(folio); return true; } return false; @@ -7488,7 +7488,7 @@ static void btrfs_invalidate_folio(struct folio *folio, size_t offset, btrfs_folio_clear_checked(fs_info, folio, folio_pos(folio), folio_size(folio)); if (!inode_evicting) __btrfs_release_folio(folio, GFP_NOFS); - clear_folio_extent_mapped(folio); + btrfs_clear_folio_subpage(folio); } static int btrfs_truncate(struct btrfs_inode *inode, bool skip_writeback) diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c index f0824c948cb7..f1060cab079f 100644 --- a/fs/btrfs/reflink.c +++ b/fs/btrfs/reflink.c @@ -91,7 +91,7 @@ static int copy_inline_to_page(struct btrfs_inode *inode, goto out_unlock; } - ret = set_folio_extent_mapped(folio); + ret = btrfs_set_folio_subpage(folio); if (ret < 0) goto out_unlock; diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c index af0969b70b53..285b0e5a9ab9 100644 --- a/fs/btrfs/relocation.c +++ b/fs/btrfs/relocation.c @@ -2873,7 +2873,7 @@ static int relocate_one_folio(struct reloc_control *rc, * folio above, make sure we set_folio_extent_mapped() here so we have any * of the subpage blocksize stuff we need in place. */ - ret = set_folio_extent_mapped(folio); + ret = btrfs_set_folio_subpage(folio); if (ret < 0) goto release_folio; -- 2.48.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] btrfs: kill EXTENT_FOLIO_PRIVATE 2025-02-21 20:20 ` [PATCH 2/2] btrfs: kill EXTENT_FOLIO_PRIVATE Goldwyn Rodrigues @ 2025-02-28 14:37 ` David Sterba 2025-03-07 11:23 ` Goldwyn Rodrigues 0 siblings, 1 reply; 6+ messages in thread From: David Sterba @ 2025-02-28 14:37 UTC (permalink / raw) To: Goldwyn Rodrigues; +Cc: linux-btrfs, Goldwyn Rodrigues On Fri, Feb 21, 2025 at 03:20:53PM -0500, Goldwyn Rodrigues wrote: > From: Goldwyn Rodrigues <rgoldwyn@suse.com> > > Since mapping_set_release_always() will provide a callback for release > folio, remove EXTENT_FOLIO_PRIVATE and all it's helper functions. > > This affects how we handle subpage, so convert the function name of > set_folio_extent_mapped() to btrfs_set_folio_subpage() and > clear_folio_extent_mapped() to btrfs_clear_folio_subpage(). > > free_space_cache_inode does not use subpage, so just skip calling > btrfs_set_folio_subpage(). > > Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com> > --- > fs/btrfs/compression.c | 2 +- > fs/btrfs/defrag.c | 2 +- > fs/btrfs/extent_io.c | 28 +++++++++++++--------------- > fs/btrfs/extent_io.h | 10 ++-------- > fs/btrfs/file.c | 4 ++-- > fs/btrfs/free-space-cache.c | 9 --------- > fs/btrfs/inode.c | 6 +++--- > fs/btrfs/reflink.c | 2 +- > fs/btrfs/relocation.c | 2 +- > 9 files changed, 24 insertions(+), 41 deletions(-) > > diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c > index 1fe154e7cc02..8f0cc726ba20 100644 > --- a/fs/btrfs/compression.c > +++ b/fs/btrfs/compression.c > @@ -491,7 +491,7 @@ static noinline int add_ra_bio_pages(struct inode *inode, > *memstall = 1; > } > > - ret = set_folio_extent_mapped(folio); > + ret = btrfs_set_folio_subpage(folio); > if (ret < 0) { > folio_unlock(folio); > folio_put(folio); > diff --git a/fs/btrfs/defrag.c b/fs/btrfs/defrag.c > index 968dae953948..46df90f9e790 100644 > --- a/fs/btrfs/defrag.c > +++ b/fs/btrfs/defrag.c > @@ -883,7 +883,7 @@ static struct folio *defrag_prepare_one_folio(struct btrfs_inode *inode, pgoff_t > return ERR_PTR(-ETXTBSY); > } > > - ret = set_folio_extent_mapped(folio); > + ret = btrfs_set_folio_subpage(folio); > if (ret < 0) { > folio_unlock(folio); > folio_put(folio); > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index d1f9fad18f25..ca29a1111de6 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -865,25 +865,22 @@ static int attach_extent_buffer_folio(struct extent_buffer *eb, > return ret; > } > > -int set_folio_extent_mapped(struct folio *folio) > +int btrfs_set_folio_subpage(struct folio *folio) > { > - struct btrfs_fs_info *fs_info; > + struct btrfs_fs_info *fs_info = folio_to_fs_info(folio); > > ASSERT(folio->mapping); > > - if (folio_test_private(folio)) > + if (!btrfs_is_subpage(fs_info, folio->mapping)) > return 0; > > - fs_info = folio_to_fs_info(folio); > - > - if (btrfs_is_subpage(fs_info, folio->mapping)) > - return btrfs_attach_subpage(fs_info, folio, BTRFS_SUBPAGE_DATA); > + if (folio_test_private(folio)) > + return 0; Is this necessary to check the private bit here? It is maybe a shortcut because btrfs_attach_subpage will test the private bit as well but I don't think this is a hot path where we'd need to optimize it. > > - folio_attach_private(folio, (void *)EXTENT_FOLIO_PRIVATE); > - return 0; > + return btrfs_attach_subpage(fs_info, folio, BTRFS_SUBPAGE_DATA); > } > > -void clear_folio_extent_mapped(struct folio *folio) > +void btrfs_clear_folio_subpage(struct folio *folio) > { > struct btrfs_fs_info *fs_info; > > @@ -893,10 +890,11 @@ void clear_folio_extent_mapped(struct folio *folio) > return; > > fs_info = folio_to_fs_info(folio); > - if (btrfs_is_subpage(fs_info, folio->mapping)) > - return btrfs_detach_subpage(fs_info, folio, BTRFS_SUBPAGE_DATA); > > - folio_detach_private(folio); > + if (!btrfs_is_subpage(fs_info, folio->mapping)) > + return; > + > + btrfs_detach_subpage(fs_info, folio, BTRFS_SUBPAGE_DATA); > } > > static struct extent_map *get_extent_map(struct btrfs_inode *inode, > @@ -951,7 +949,7 @@ static int btrfs_do_readpage(struct folio *folio, struct extent_map **em_cached, > size_t iosize; > size_t blocksize = fs_info->sectorsize; > > - ret = set_folio_extent_mapped(folio); > + ret = btrfs_set_folio_subpage(folio); > if (ret < 0) { > folio_unlock(folio); > return ret; > @@ -1562,7 +1560,7 @@ static int extent_writepage(struct folio *folio, struct btrfs_bio_ctrl *bio_ctrl > * The proper bitmap can only be initialized until writepage_delalloc(). > */ > bio_ctrl->submit_bitmap = (unsigned long)-1; > - ret = set_folio_extent_mapped(folio); > + ret = btrfs_set_folio_subpage(folio); > if (ret < 0) > goto done; > > diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h > index 6c5328bfabc2..303c92272a46 100644 > --- a/fs/btrfs/extent_io.h > +++ b/fs/btrfs/extent_io.h > @@ -65,12 +65,6 @@ enum { > ENUM_BIT(PAGE_SET_ORDERED), > }; > > -/* > - * Folio private values. Every page that is controlled by the extent map has > - * folio private set to this value. > - */ > -#define EXTENT_FOLIO_PRIVATE 1 The whole naming and extent mapping is from the old times where the relation between page and extent depended on the private bit but it probably got changed during the folio conversion so we don't really need it anymore. It still looks strange to see btrfs_set_folio_subpage() at random place that do not seem to be related to subpage, so the "mapped" semantics fits better but as said it's probably not correct anymore. > - > /* > * The extent buffer bitmap operations are done with byte granularity instead of > * word granularity for two reasons: > @@ -247,8 +241,8 @@ int btrfs_writepages(struct address_space *mapping, struct writeback_control *wb > int btree_write_cache_pages(struct address_space *mapping, > struct writeback_control *wbc); > void btrfs_readahead(struct readahead_control *rac); > -int set_folio_extent_mapped(struct folio *folio); > -void clear_folio_extent_mapped(struct folio *folio); > +int btrfs_set_folio_subpage(struct folio *folio); > +void btrfs_clear_folio_subpage(struct folio *folio); > > struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info, > u64 start, u64 owner_root, int level); > diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c > index 5808eb5bcd42..820feaf26583 100644 > --- a/fs/btrfs/file.c > +++ b/fs/btrfs/file.c > @@ -875,7 +875,7 @@ static noinline int prepare_one_folio(struct inode *inode, struct folio **folio_ > } > /* Only support page sized folio yet. */ > ASSERT(folio_order(folio) == 0); > - ret = set_folio_extent_mapped(folio); > + ret = btrfs_set_folio_subpage(folio); > if (ret < 0) { > folio_unlock(folio); > folio_put(folio); > @@ -1840,7 +1840,7 @@ static vm_fault_t btrfs_page_mkwrite(struct vm_fault *vmf) > folio_wait_writeback(folio); > > lock_extent(io_tree, page_start, page_end, &cached_state); > - ret2 = set_folio_extent_mapped(folio); > + ret2 = btrfs_set_folio_subpage(folio); > if (ret2 < 0) { > ret = vmf_error(ret2); > unlock_extent(io_tree, page_start, page_end, &cached_state); > diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c > index 056546bf9abd..7a85b243f18e 100644 > --- a/fs/btrfs/free-space-cache.c > +++ b/fs/btrfs/free-space-cache.c > @@ -453,7 +453,6 @@ static int io_ctl_prepare_pages(struct btrfs_io_ctl *io_ctl, bool uptodate) > int i; > > for (i = 0; i < io_ctl->num_pages; i++) { > - int ret; > > folio = __filemap_get_folio(inode->i_mapping, i, > FGP_LOCK | FGP_ACCESSED | FGP_CREAT, > @@ -463,14 +462,6 @@ static int io_ctl_prepare_pages(struct btrfs_io_ctl *io_ctl, bool uptodate) > return -ENOMEM; > } > > - ret = set_folio_extent_mapped(folio); > - if (ret < 0) { > - folio_unlock(folio); > - folio_put(folio); > - io_ctl_drop_pages(io_ctl); > - return ret; > - } > - > io_ctl->pages[i] = &folio->page; > if (uptodate && !folio_test_uptodate(folio)) { > btrfs_read_folio(NULL, folio); > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index 6424d45c6baa..896c9454dc96 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -4863,7 +4863,7 @@ int btrfs_truncate_block(struct btrfs_inode *inode, loff_t from, loff_t len, > * folio private, but left the page in the mapping. Set the page mapped > * here to make sure it's properly set for the subpage stuff. > */ > - ret = set_folio_extent_mapped(folio); > + ret = btrfs_set_folio_subpage(folio); > if (ret < 0) > goto out_unlock; > > @@ -7290,7 +7290,7 @@ static bool __btrfs_release_folio(struct folio *folio, gfp_t gfp_flags) > { > if (try_release_extent_mapping(folio, gfp_flags)) { > wait_subpage_spinlock(folio); > - clear_folio_extent_mapped(folio); > + btrfs_clear_folio_subpage(folio); > return true; > } > return false; > @@ -7488,7 +7488,7 @@ static void btrfs_invalidate_folio(struct folio *folio, size_t offset, > btrfs_folio_clear_checked(fs_info, folio, folio_pos(folio), folio_size(folio)); > if (!inode_evicting) > __btrfs_release_folio(folio, GFP_NOFS); > - clear_folio_extent_mapped(folio); > + btrfs_clear_folio_subpage(folio); > } > > static int btrfs_truncate(struct btrfs_inode *inode, bool skip_writeback) > diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c > index f0824c948cb7..f1060cab079f 100644 > --- a/fs/btrfs/reflink.c > +++ b/fs/btrfs/reflink.c > @@ -91,7 +91,7 @@ static int copy_inline_to_page(struct btrfs_inode *inode, > goto out_unlock; > } > > - ret = set_folio_extent_mapped(folio); > + ret = btrfs_set_folio_subpage(folio); > if (ret < 0) > goto out_unlock; > > diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c > index af0969b70b53..285b0e5a9ab9 100644 > --- a/fs/btrfs/relocation.c > +++ b/fs/btrfs/relocation.c > @@ -2873,7 +2873,7 @@ static int relocate_one_folio(struct reloc_control *rc, > * folio above, make sure we set_folio_extent_mapped() here so we have any Leftover set_folio_extent_mapped() in the comment. > * of the subpage blocksize stuff we need in place. > */ > - ret = set_folio_extent_mapped(folio); > + ret = btrfs_set_folio_subpage(folio); > if (ret < 0) > goto release_folio; > > -- > 2.48.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] btrfs: kill EXTENT_FOLIO_PRIVATE 2025-02-28 14:37 ` David Sterba @ 2025-03-07 11:23 ` Goldwyn Rodrigues 0 siblings, 0 replies; 6+ messages in thread From: Goldwyn Rodrigues @ 2025-03-07 11:23 UTC (permalink / raw) To: David Sterba; +Cc: linux-btrfs On 15:37 28/02, David Sterba wrote: > On Fri, Feb 21, 2025 at 03:20:53PM -0500, Goldwyn Rodrigues wrote: > > From: Goldwyn Rodrigues <rgoldwyn@suse.com> > > > > Since mapping_set_release_always() will provide a callback for release > > folio, remove EXTENT_FOLIO_PRIVATE and all it's helper functions. > > > > This affects how we handle subpage, so convert the function name of > > set_folio_extent_mapped() to btrfs_set_folio_subpage() and > > clear_folio_extent_mapped() to btrfs_clear_folio_subpage(). > > > > free_space_cache_inode does not use subpage, so just skip calling > > btrfs_set_folio_subpage(). > > > > Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com> > > --- > > fs/btrfs/compression.c | 2 +- > > fs/btrfs/defrag.c | 2 +- > > fs/btrfs/extent_io.c | 28 +++++++++++++--------------- > > fs/btrfs/extent_io.h | 10 ++-------- > > fs/btrfs/file.c | 4 ++-- > > fs/btrfs/free-space-cache.c | 9 --------- > > fs/btrfs/inode.c | 6 +++--- > > fs/btrfs/reflink.c | 2 +- > > fs/btrfs/relocation.c | 2 +- > > 9 files changed, 24 insertions(+), 41 deletions(-) > > > > diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c > > index 1fe154e7cc02..8f0cc726ba20 100644 > > --- a/fs/btrfs/compression.c > > +++ b/fs/btrfs/compression.c > > @@ -491,7 +491,7 @@ static noinline int add_ra_bio_pages(struct inode *inode, > > *memstall = 1; > > } > > > > - ret = set_folio_extent_mapped(folio); > > + ret = btrfs_set_folio_subpage(folio); > > if (ret < 0) { > > folio_unlock(folio); > > folio_put(folio); > > diff --git a/fs/btrfs/defrag.c b/fs/btrfs/defrag.c > > index 968dae953948..46df90f9e790 100644 > > --- a/fs/btrfs/defrag.c > > +++ b/fs/btrfs/defrag.c > > @@ -883,7 +883,7 @@ static struct folio *defrag_prepare_one_folio(struct btrfs_inode *inode, pgoff_t > > return ERR_PTR(-ETXTBSY); > > } > > > > - ret = set_folio_extent_mapped(folio); > > + ret = btrfs_set_folio_subpage(folio); > > if (ret < 0) { > > folio_unlock(folio); > > folio_put(folio); > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > > index d1f9fad18f25..ca29a1111de6 100644 > > --- a/fs/btrfs/extent_io.c > > +++ b/fs/btrfs/extent_io.c > > @@ -865,25 +865,22 @@ static int attach_extent_buffer_folio(struct extent_buffer *eb, > > return ret; > > } > > > > -int set_folio_extent_mapped(struct folio *folio) > > +int btrfs_set_folio_subpage(struct folio *folio) > > { > > - struct btrfs_fs_info *fs_info; > > + struct btrfs_fs_info *fs_info = folio_to_fs_info(folio); > > > > ASSERT(folio->mapping); > > > > - if (folio_test_private(folio)) > > + if (!btrfs_is_subpage(fs_info, folio->mapping)) > > return 0; > > > > - fs_info = folio_to_fs_info(folio); > > - > > - if (btrfs_is_subpage(fs_info, folio->mapping)) > > - return btrfs_attach_subpage(fs_info, folio, BTRFS_SUBPAGE_DATA); > > + if (folio_test_private(folio)) > > + return 0; > > Is this necessary to check the private bit here? It is maybe a shortcut > because btrfs_attach_subpage will test the private bit as well but I > don't think this is a hot path where we'd need to optimize it. No. As a matter of fact, we can remove btrfs_is_subpage() check as well since this is checked in btrfs_attach_subpage(). > > > > > - folio_attach_private(folio, (void *)EXTENT_FOLIO_PRIVATE); > > - return 0; > > + return btrfs_attach_subpage(fs_info, folio, BTRFS_SUBPAGE_DATA); > > } > > > > -void clear_folio_extent_mapped(struct folio *folio) > > +void btrfs_clear_folio_subpage(struct folio *folio) > > { > > struct btrfs_fs_info *fs_info; > > > > @@ -893,10 +890,11 @@ void clear_folio_extent_mapped(struct folio *folio) > > return; > > > > fs_info = folio_to_fs_info(folio); > > - if (btrfs_is_subpage(fs_info, folio->mapping)) > > - return btrfs_detach_subpage(fs_info, folio, BTRFS_SUBPAGE_DATA); > > > > - folio_detach_private(folio); > > + if (!btrfs_is_subpage(fs_info, folio->mapping)) > > + return; > > + > > + btrfs_detach_subpage(fs_info, folio, BTRFS_SUBPAGE_DATA); > > } > > > > static struct extent_map *get_extent_map(struct btrfs_inode *inode, > > @@ -951,7 +949,7 @@ static int btrfs_do_readpage(struct folio *folio, struct extent_map **em_cached, > > size_t iosize; > > size_t blocksize = fs_info->sectorsize; > > > > - ret = set_folio_extent_mapped(folio); > > + ret = btrfs_set_folio_subpage(folio); > > if (ret < 0) { > > folio_unlock(folio); > > return ret; > > @@ -1562,7 +1560,7 @@ static int extent_writepage(struct folio *folio, struct btrfs_bio_ctrl *bio_ctrl > > * The proper bitmap can only be initialized until writepage_delalloc(). > > */ > > bio_ctrl->submit_bitmap = (unsigned long)-1; > > - ret = set_folio_extent_mapped(folio); > > + ret = btrfs_set_folio_subpage(folio); > > if (ret < 0) > > goto done; > > > > diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h > > index 6c5328bfabc2..303c92272a46 100644 > > --- a/fs/btrfs/extent_io.h > > +++ b/fs/btrfs/extent_io.h > > @@ -65,12 +65,6 @@ enum { > > ENUM_BIT(PAGE_SET_ORDERED), > > }; > > > > -/* > > - * Folio private values. Every page that is controlled by the extent map has > > - * folio private set to this value. > > - */ > > -#define EXTENT_FOLIO_PRIVATE 1 > > The whole naming and extent mapping is from the old times where the > relation between page and extent depended on the private bit but it > probably got changed during the folio conversion so we don't really need > it anymore. > > It still looks strange to see btrfs_set_folio_subpage() at random place > that do not seem to be related to subpage, so the "mapped" semantics > fits better but as said it's probably not correct anymore. > I am not sure how to improve it. A better name for the function? > > - > > /* > > * The extent buffer bitmap operations are done with byte granularity instead of > > * word granularity for two reasons: > > @@ -247,8 +241,8 @@ int btrfs_writepages(struct address_space *mapping, struct writeback_control *wb > > int btree_write_cache_pages(struct address_space *mapping, > > struct writeback_control *wbc); > > void btrfs_readahead(struct readahead_control *rac); > > -int set_folio_extent_mapped(struct folio *folio); > > -void clear_folio_extent_mapped(struct folio *folio); > > +int btrfs_set_folio_subpage(struct folio *folio); > > +void btrfs_clear_folio_subpage(struct folio *folio); > > > > struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info, > > u64 start, u64 owner_root, int level); > > diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c > > index 5808eb5bcd42..820feaf26583 100644 > > --- a/fs/btrfs/file.c > > +++ b/fs/btrfs/file.c > > @@ -875,7 +875,7 @@ static noinline int prepare_one_folio(struct inode *inode, struct folio **folio_ > > } > > /* Only support page sized folio yet. */ > > ASSERT(folio_order(folio) == 0); > > - ret = set_folio_extent_mapped(folio); > > + ret = btrfs_set_folio_subpage(folio); > > if (ret < 0) { > > folio_unlock(folio); > > folio_put(folio); > > @@ -1840,7 +1840,7 @@ static vm_fault_t btrfs_page_mkwrite(struct vm_fault *vmf) > > folio_wait_writeback(folio); > > > > lock_extent(io_tree, page_start, page_end, &cached_state); > > - ret2 = set_folio_extent_mapped(folio); > > + ret2 = btrfs_set_folio_subpage(folio); > > if (ret2 < 0) { > > ret = vmf_error(ret2); > > unlock_extent(io_tree, page_start, page_end, &cached_state); > > diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c > > index 056546bf9abd..7a85b243f18e 100644 > > --- a/fs/btrfs/free-space-cache.c > > +++ b/fs/btrfs/free-space-cache.c > > @@ -453,7 +453,6 @@ static int io_ctl_prepare_pages(struct btrfs_io_ctl *io_ctl, bool uptodate) > > int i; > > > > for (i = 0; i < io_ctl->num_pages; i++) { > > - int ret; > > > > folio = __filemap_get_folio(inode->i_mapping, i, > > FGP_LOCK | FGP_ACCESSED | FGP_CREAT, > > @@ -463,14 +462,6 @@ static int io_ctl_prepare_pages(struct btrfs_io_ctl *io_ctl, bool uptodate) > > return -ENOMEM; > > } > > > > - ret = set_folio_extent_mapped(folio); > > - if (ret < 0) { > > - folio_unlock(folio); > > - folio_put(folio); > > - io_ctl_drop_pages(io_ctl); > > - return ret; > > - } > > - > > io_ctl->pages[i] = &folio->page; > > if (uptodate && !folio_test_uptodate(folio)) { > > btrfs_read_folio(NULL, folio); > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > > index 6424d45c6baa..896c9454dc96 100644 > > --- a/fs/btrfs/inode.c > > +++ b/fs/btrfs/inode.c > > @@ -4863,7 +4863,7 @@ int btrfs_truncate_block(struct btrfs_inode *inode, loff_t from, loff_t len, > > * folio private, but left the page in the mapping. Set the page mapped > > * here to make sure it's properly set for the subpage stuff. > > */ > > - ret = set_folio_extent_mapped(folio); > > + ret = btrfs_set_folio_subpage(folio); > > if (ret < 0) > > goto out_unlock; > > > > @@ -7290,7 +7290,7 @@ static bool __btrfs_release_folio(struct folio *folio, gfp_t gfp_flags) > > { > > if (try_release_extent_mapping(folio, gfp_flags)) { > > wait_subpage_spinlock(folio); > > - clear_folio_extent_mapped(folio); > > + btrfs_clear_folio_subpage(folio); > > return true; > > } > > return false; > > @@ -7488,7 +7488,7 @@ static void btrfs_invalidate_folio(struct folio *folio, size_t offset, > > btrfs_folio_clear_checked(fs_info, folio, folio_pos(folio), folio_size(folio)); > > if (!inode_evicting) > > __btrfs_release_folio(folio, GFP_NOFS); > > - clear_folio_extent_mapped(folio); > > + btrfs_clear_folio_subpage(folio); > > } > > > > static int btrfs_truncate(struct btrfs_inode *inode, bool skip_writeback) > > diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c > > index f0824c948cb7..f1060cab079f 100644 > > --- a/fs/btrfs/reflink.c > > +++ b/fs/btrfs/reflink.c > > @@ -91,7 +91,7 @@ static int copy_inline_to_page(struct btrfs_inode *inode, > > goto out_unlock; > > } > > > > - ret = set_folio_extent_mapped(folio); > > + ret = btrfs_set_folio_subpage(folio); > > if (ret < 0) > > goto out_unlock; > > > > diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c > > index af0969b70b53..285b0e5a9ab9 100644 > > --- a/fs/btrfs/relocation.c > > +++ b/fs/btrfs/relocation.c > > @@ -2873,7 +2873,7 @@ static int relocate_one_folio(struct reloc_control *rc, > > * folio above, make sure we set_folio_extent_mapped() here so we have any > > Leftover set_folio_extent_mapped() in the comment. > > > * of the subpage blocksize stuff we need in place. > > */ > > - ret = set_folio_extent_mapped(folio); > > + ret = btrfs_set_folio_subpage(folio); > > if (ret < 0) > > goto release_folio; > > > > -- > > 2.48.1 > > -- Goldwyn ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-03-07 11:23 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-02-21 20:20 [PATCH 0/2] Avoid setting folio->private Goldwyn Rodrigues 2025-02-21 20:20 ` [PATCH 1/2] btrfs: add mapping_set_release_always to inode's mapping Goldwyn Rodrigues 2025-02-28 14:23 ` David Sterba 2025-02-21 20:20 ` [PATCH 2/2] btrfs: kill EXTENT_FOLIO_PRIVATE Goldwyn Rodrigues 2025-02-28 14:37 ` David Sterba 2025-03-07 11:23 ` Goldwyn Rodrigues
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox