Re: [PATCH RFC v2 for-6.8/block 01/18] block: add some bdev apis
Hi, 在 2023/12/12 21:14, Christoph Hellwig 写道: On Mon, Dec 11, 2023 at 05:52:17PM +0100, Jan Kara wrote: +void bdev_associated_mapping(struct block_device *bdev, +struct address_space *mapping) +{ + mapping->host = bdev->bd_inode; +} Here I'm not sure - is the helper really a win? It seems a bit obscure to me. This initialization of another mapping for a bdev looks really special. If we want to hide bd_inode we'll something like this helper even if I don't particularly like it either. But it might be a good idea to move out of this series and into the follow on removing bd_inode, as it's rather pointless without that context. Yes, this sounds good, I'll remove this from v3. Thanks, Kuai .
Re: [PATCH RFC v2 for-6.8/block 01/18] block: add some bdev apis
Hi, 在 2023/12/12 21:16, Christoph Hellwig 写道: +void invalidate_bdev_range(struct block_device *bdev, pgoff_t start, + pgoff_t end) +{ + invalidate_mapping_pages(bdev->bd_inode->i_mapping, start, end); +} +EXPORT_SYMBOL_GPL(invalidate_bdev_range); Can we have kerneldoc comments for the new helpers please? Of course, will definitely do this in v3. +struct folio *__bdev_get_folio(struct block_device *bdev, loff_t pos, + fgf_t fgp_flags, gfp_t gfp) +{ + return __filemap_get_folio(bdev->bd_inode->i_mapping, pos >> PAGE_SHIFT, + fgp_flags, gfp); +} +EXPORT_SYMBOL_GPL(__bdev_get_folio); It's a bit silly to have a __-prefixed API without a version that doesn't have the prefix, so I'd prefer to drop it. Unless willy has a good argument for keeping it the same as the filemap API. Ok, I'll drop it if willy doesn't against this. Thanks, Kuai .
Re: [PATCH RFC v2 for-6.8/block 01/18] block: add some bdev apis
> +void invalidate_bdev_range(struct block_device *bdev, pgoff_t start, > +pgoff_t end) > +{ > + invalidate_mapping_pages(bdev->bd_inode->i_mapping, start, end); > +} > +EXPORT_SYMBOL_GPL(invalidate_bdev_range); Can we have kerneldoc comments for the new helpers please? > +struct folio *__bdev_get_folio(struct block_device *bdev, loff_t pos, > +fgf_t fgp_flags, gfp_t gfp) > +{ > + return __filemap_get_folio(bdev->bd_inode->i_mapping, pos >> PAGE_SHIFT, > +fgp_flags, gfp); > +} > +EXPORT_SYMBOL_GPL(__bdev_get_folio); It's a bit silly to have a __-prefixed API without a version that doesn't have the prefix, so I'd prefer to drop it. Unless willy has a good argument for keeping it the same as the filemap API.
Re: [PATCH RFC v2 for-6.8/block 01/18] block: add some bdev apis
On Mon, Dec 11, 2023 at 05:52:17PM +0100, Jan Kara wrote: > > +void bdev_associated_mapping(struct block_device *bdev, > > +struct address_space *mapping) > > +{ > > + mapping->host = bdev->bd_inode; > > +} > > Here I'm not sure - is the helper really a win? It seems a bit obscure to > me. This initialization of another mapping for a bdev looks really special. If we want to hide bd_inode we'll something like this helper even if I don't particularly like it either. But it might be a good idea to move out of this series and into the follow on removing bd_inode, as it's rather pointless without that context.
Re: [PATCH RFC v2 for-6.8/block 01/18] block: add some bdev apis
Hi, 在 2023/12/12 0:52, Jan Kara 写道: On Mon 11-12-23 22:05:35, Yu Kuai wrote: From: Yu Kuai Those apis will be used for other modules, so that bd_inode won't be accessed directly from other modules. Signed-off-by: Yu Kuai ... +void bdev_associated_mapping(struct block_device *bdev, +struct address_space *mapping) +{ + mapping->host = bdev->bd_inode; +} Here I'm not sure - is the helper really a win? It seems a bit obscure to me. This initialization of another mapping for a bdev looks really special. Yes, I don't like this helper at all, but gfs2 is used this way, and I need this helper to remove 'bd_inode' from block_devcie later. I'm not familiar with gfs2 at all but perhaps it worth to dig deeper and figure out a proper way for gfs2. Thanks, Kuai Honza
Re: [PATCH RFC v2 for-6.8/block 01/18] block: add some bdev apis
On Mon 11-12-23 22:05:35, Yu Kuai wrote: > From: Yu Kuai > > Those apis will be used for other modules, so that bd_inode won't be > accessed directly from other modules. > > Signed-off-by: Yu Kuai ... > +void bdev_associated_mapping(struct block_device *bdev, > + struct address_space *mapping) > +{ > + mapping->host = bdev->bd_inode; > +} Here I'm not sure - is the helper really a win? It seems a bit obscure to me. This initialization of another mapping for a bdev looks really special. Honza -- Jan Kara SUSE Labs, CR
[PATCH RFC v2 for-6.8/block 01/18] block: add some bdev apis
From: Yu Kuai Those apis will be used for other modules, so that bd_inode won't be accessed directly from other modules. Signed-off-by: Yu Kuai --- block/bdev.c | 70 ++ block/blk.h| 2 -- include/linux/blkdev.h | 17 ++ 3 files changed, 87 insertions(+), 2 deletions(-) diff --git a/block/bdev.c b/block/bdev.c index 750aec178b6a..9a469753eb4b 100644 --- a/block/bdev.c +++ b/block/bdev.c @@ -89,6 +89,13 @@ void invalidate_bdev(struct block_device *bdev) } EXPORT_SYMBOL(invalidate_bdev); +void invalidate_bdev_range(struct block_device *bdev, pgoff_t start, + pgoff_t end) +{ + invalidate_mapping_pages(bdev->bd_inode->i_mapping, start, end); +} +EXPORT_SYMBOL_GPL(invalidate_bdev_range); + /* * Drop all buffers & page cache for given bdev range. This function bails * with error if bdev has other exclusive owner (such as filesystem). @@ -121,6 +128,7 @@ int truncate_bdev_range(struct block_device *bdev, blk_mode_t mode, lstart >> PAGE_SHIFT, lend >> PAGE_SHIFT); } +EXPORT_SYMBOL_GPL(truncate_bdev_range); static void set_init_blocksize(struct block_device *bdev) { @@ -1102,3 +1110,65 @@ void bdev_statx_dioalign(struct inode *inode, struct kstat *stat) blkdev_put_no_open(bdev); } + +struct folio *bdev_read_folio(struct block_device *bdev, loff_t pos) +{ + return mapping_read_folio_gfp(bdev->bd_inode->i_mapping, + pos >> PAGE_SHIFT, GFP_KERNEL); +} +EXPORT_SYMBOL_GPL(bdev_read_folio); + +struct folio *__bdev_get_folio(struct block_device *bdev, loff_t pos, + fgf_t fgp_flags, gfp_t gfp) +{ + return __filemap_get_folio(bdev->bd_inode->i_mapping, pos >> PAGE_SHIFT, + fgp_flags, gfp); +} +EXPORT_SYMBOL_GPL(__bdev_get_folio); + +int bdev_wb_err_check(struct block_device *bdev, errseq_t since) +{ + return errseq_check(&bdev->bd_inode->i_mapping->wb_err, since); +} +EXPORT_SYMBOL_GPL(bdev_wb_err_check); + +int bdev_wb_err_check_and_advance(struct block_device *bdev, errseq_t *since) +{ + return errseq_check_and_advance(&bdev->bd_inode->i_mapping->wb_err, + since); +} +EXPORT_SYMBOL_GPL(bdev_wb_err_check_and_advance); + +void bdev_balance_dirty_pages_ratelimited(struct block_device *bdev) +{ + return balance_dirty_pages_ratelimited(bdev->bd_inode->i_mapping); +} +EXPORT_SYMBOL_GPL(bdev_balance_dirty_pages_ratelimited); + +void bdev_sync_readahead(struct block_device *bdev, struct file_ra_state *ra, +struct file *file, pgoff_t index, +unsigned long req_count) +{ + struct file_ra_state tmp_ra = {}; + + if (!ra) { + ra = &tmp_ra; + file_ra_state_init(ra, bdev->bd_inode->i_mapping); + } + page_cache_sync_readahead(bdev->bd_inode->i_mapping, ra, file, index, + req_count); +} +EXPORT_SYMBOL_GPL(bdev_sync_readahead); + +void bdev_attach_wb(struct block_device *bdev) +{ + inode_attach_wb(bdev->bd_inode, NULL); +} +EXPORT_SYMBOL_GPL(bdev_attach_wb); + +void bdev_associated_mapping(struct block_device *bdev, +struct address_space *mapping) +{ + mapping->host = bdev->bd_inode; +} +EXPORT_SYMBOL_GPL(bdev_associated_mapping); diff --git a/block/blk.h b/block/blk.h index 08a358bc0919..da4becd4f7e9 100644 --- a/block/blk.h +++ b/block/blk.h @@ -467,8 +467,6 @@ extern struct device_attribute dev_attr_events_poll_msecs; extern struct attribute_group blk_trace_attr_group; blk_mode_t file_to_blk_mode(struct file *file); -int truncate_bdev_range(struct block_device *bdev, blk_mode_t mode, - loff_t lstart, loff_t lend); long blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg); long compat_blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg); diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 17c0a7d0d319..d2453424a9eb 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -24,6 +24,7 @@ #include #include #include +#include struct module; struct request_queue; @@ -1502,6 +1503,22 @@ struct block_device *blkdev_get_no_open(dev_t dev); void blkdev_put_no_open(struct block_device *bdev); struct block_device *I_BDEV(struct inode *inode); +void invalidate_bdev_range(struct block_device *bdev, pgoff_t start, + pgoff_t end); +int truncate_bdev_range(struct block_device *bdev, blk_mode_t mode, + loff_t lstart, loff_t lend); +struct folio *bdev_read_folio(struct block_device *bdev, loff_t pos); +struct folio *__bdev_get_folio(struct block_device *bdev, loff_t pos, + fgf_t fgp_flags, gfp_t gfp); +int bdev_wb_err_check(struct block_device *bdev, e