| Message ID | 86448b99cc16046569c38cdef83c41afcd8047ed.1706553080.git.dsterba@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Struct to fs_info helpers | expand |
On 29.01.24 19:33, David Sterba wrote: > diff --git a/fs/btrfs/misc.h b/fs/btrfs/misc.h > index 40f2d9f1a17a..8be09234c575 100644 > --- a/fs/btrfs/misc.h > +++ b/fs/btrfs/misc.h > @@ -8,6 +8,9 @@ > #include <linux/math64.h> > #include <linux/rbtree.h> > > +#define page_to_inode(page) BTRFS_I((page)->mapping->host) > +#define folio_to_inode(folio) BTRFS_I((folio)->mapping->host) > + Why are you using a macro instead of an inline function here? Shouldn't inline function give us a bit more type safety, or are compilers smart enough nowadays?
On Tue, Jan 30, 2024 at 11:42:30AM +0000, Johannes Thumshirn wrote: > On 29.01.24 19:33, David Sterba wrote: > > diff --git a/fs/btrfs/misc.h b/fs/btrfs/misc.h > > index 40f2d9f1a17a..8be09234c575 100644 > > --- a/fs/btrfs/misc.h > > +++ b/fs/btrfs/misc.h > > @@ -8,6 +8,9 @@ > > #include <linux/math64.h> > > #include <linux/rbtree.h> > > > > +#define page_to_inode(page) BTRFS_I((page)->mapping->host) > > +#define folio_to_inode(folio) BTRFS_I((folio)->mapping->host) > > + > > Why are you using a macro instead of an inline function here? As said in the changelog so we don't have to include headers with full definitions of page, folio, fs_info, ... > Shouldn't inline function give us a bit more type safety, or are > compilers smart enough nowadays? Yes type safety would be good but then it can't be an inline without bloating misc.h (and the making include cycles). I could do a type check in the macro using _Generic.
On 30.01.24 20:29, David Sterba wrote: > On Tue, Jan 30, 2024 at 11:42:30AM +0000, Johannes Thumshirn wrote: >> On 29.01.24 19:33, David Sterba wrote: >>> diff --git a/fs/btrfs/misc.h b/fs/btrfs/misc.h >>> index 40f2d9f1a17a..8be09234c575 100644 >>> --- a/fs/btrfs/misc.h >>> +++ b/fs/btrfs/misc.h >>> @@ -8,6 +8,9 @@ >>> #include <linux/math64.h> >>> #include <linux/rbtree.h> >>> >>> +#define page_to_inode(page) BTRFS_I((page)->mapping->host) >>> +#define folio_to_inode(folio) BTRFS_I((folio)->mapping->host) >>> + >> >> Why are you using a macro instead of an inline function here? > > As said in the changelog so we don't have to include headers with full > definitions of page, folio, fs_info, ... > >> Shouldn't inline function give us a bit more type safety, or are >> compilers smart enough nowadays? > > Yes type safety would be good but then it can't be an inline without > bloating misc.h (and the making include cycles). I personally would've put them into fs.h anyways. > > I could do a type check in the macro using _Generic. >
On Wed, Jan 31, 2024 at 09:33:58AM +0000, Johannes Thumshirn wrote: > On 30.01.24 20:29, David Sterba wrote: > > On Tue, Jan 30, 2024 at 11:42:30AM +0000, Johannes Thumshirn wrote: > >> On 29.01.24 19:33, David Sterba wrote: > >>> diff --git a/fs/btrfs/misc.h b/fs/btrfs/misc.h > >>> index 40f2d9f1a17a..8be09234c575 100644 > >>> --- a/fs/btrfs/misc.h > >>> +++ b/fs/btrfs/misc.h > >>> @@ -8,6 +8,9 @@ > >>> #include <linux/math64.h> > >>> #include <linux/rbtree.h> > >>> > >>> +#define page_to_inode(page) BTRFS_I((page)->mapping->host) > >>> +#define folio_to_inode(folio) BTRFS_I((folio)->mapping->host) > >>> + > >> > >> Why are you using a macro instead of an inline function here? > > > > As said in the changelog so we don't have to include headers with full > > definitions of page, folio, fs_info, ... > > > >> Shouldn't inline function give us a bit more type safety, or are > >> compilers smart enough nowadays? > > > > Yes type safety would be good but then it can't be an inline without > > bloating misc.h (and the making include cycles). > > I personally would've put them into fs.h anyways. Right, that's more suitable, misc.h is for all the non-fs things.
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 26c11fce5e4e..e711bfe4d221 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -528,7 +528,8 @@ static void btree_invalidate_folio(struct folio *folio, size_t offset, size_t length) { struct extent_io_tree *tree; - tree = &BTRFS_I(folio->mapping->host)->io_tree; + + tree = &folio_to_inode(folio)->io_tree; extent_invalidate_folio(tree, folio, offset); btree_release_folio(folio, GFP_NOFS); if (folio_get_private(folio)) { diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index baa149f2152c..46667c9d61a6 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -839,7 +839,7 @@ static void submit_extent_page(struct btrfs_bio_ctrl *bio_ctrl, u64 disk_bytenr, struct page *page, size_t size, unsigned long pg_offset) { - struct btrfs_inode *inode = BTRFS_I(page->mapping->host); + struct btrfs_inode *inode = page_to_inode(page); ASSERT(pg_offset + size <= PAGE_SIZE); ASSERT(bio_ctrl->end_io_func); @@ -1171,7 +1171,7 @@ static int btrfs_do_readpage(struct page *page, struct extent_map **em_cached, int btrfs_read_folio(struct file *file, struct folio *folio) { struct page *page = &folio->page; - struct btrfs_inode *inode = BTRFS_I(page->mapping->host); + struct btrfs_inode *inode = page_to_inode(page); u64 start = page_offset(page); u64 end = start + PAGE_SIZE - 1; struct btrfs_bio_ctrl bio_ctrl = { .opf = REQ_OP_READ }; @@ -1194,7 +1194,7 @@ static inline void contiguous_readpages(struct page *pages[], int nr_pages, struct btrfs_bio_ctrl *bio_ctrl, u64 *prev_em_start) { - struct btrfs_inode *inode = BTRFS_I(pages[0]->mapping->host); + struct btrfs_inode *inode = page_to_inode(pages[0]); int index; btrfs_lock_and_flush_ordered_range(inode, start, end, NULL); @@ -2392,7 +2392,7 @@ int try_release_extent_mapping(struct page *page, gfp_t mask) struct extent_map *em; u64 start = page_offset(page); u64 end = start + PAGE_SIZE - 1; - struct btrfs_inode *btrfs_inode = BTRFS_I(page->mapping->host); + struct btrfs_inode *btrfs_inode = page_to_inode(page); struct extent_io_tree *tree = &btrfs_inode->io_tree; struct extent_map_tree *map = &btrfs_inode->extent_tree; diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index b2d348c9c93b..2d3e5359d067 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -7936,7 +7936,7 @@ static int btrfs_migrate_folio(struct address_space *mapping, static void btrfs_invalidate_folio(struct folio *folio, size_t offset, size_t length) { - struct btrfs_inode *inode = BTRFS_I(folio->mapping->host); + struct btrfs_inode *inode = folio_to_inode(folio); struct btrfs_fs_info *fs_info = inode->root->fs_info; struct extent_io_tree *tree = &inode->io_tree; struct extent_state *cached_state = NULL; diff --git a/fs/btrfs/misc.h b/fs/btrfs/misc.h index 40f2d9f1a17a..8be09234c575 100644 --- a/fs/btrfs/misc.h +++ b/fs/btrfs/misc.h @@ -8,6 +8,9 @@ #include <linux/math64.h> #include <linux/rbtree.h> +#define page_to_inode(page) BTRFS_I((page)->mapping->host) +#define folio_to_inode(folio) BTRFS_I((folio)->mapping->host) + /* * Enumerate bits using enum autoincrement. Define the @name as the n-th bit. */
Add convenience helpers to get a struct btrfs_inode from a page or folio pointer instead of open coding the chain or intermediate BTRFS_I. This is implemented as a macro so we don't need full definitions of struct page or address_space. Signed-off-by: David Sterba <dsterba@suse.com> --- fs/btrfs/disk-io.c | 3 ++- fs/btrfs/extent_io.c | 8 ++++---- fs/btrfs/inode.c | 2 +- fs/btrfs/misc.h | 3 +++ 4 files changed, 10 insertions(+), 6 deletions(-)