| Message ID | 20231211140552.973290-2-yukuai1@huaweicloud.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | block: don't access bd_inode directly from other modules | expand |
On Mon 11-12-23 22:05:35, Yu Kuai wrote: > From: Yu Kuai <yukuai3@huawei.com> > > 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 <yukuai3@huawei.com> ... > +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
Hi, 在 2023/12/12 0:52, Jan Kara 写道: > On Mon 11-12-23 22:05:35, Yu Kuai wrote: >> From: Yu Kuai <yukuai3@huawei.com> >> >> 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 <yukuai3@huawei.com> > > ... > >> +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 >
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.
> +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.
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 > > . >
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 > . >
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 <linux/sbitmap.h> #include <linux/uuid.h> #include <linux/xarray.h> +#include <linux/pagemap.h> 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, errseq_t since); +int bdev_wb_err_check_and_advance(struct block_device *bdev, errseq_t *since); +void bdev_balance_dirty_pages_ratelimited(struct block_device *bdev); +void bdev_sync_readahead(struct block_device *bdev, struct file_ra_state *ra, + struct file *file, pgoff_t index, + unsigned long req_count); +void bdev_attach_wb(struct block_device *bdev); +void bdev_associated_mapping(struct block_device *bdev, + struct address_space *mapping); #ifdef CONFIG_BLOCK void invalidate_bdev(struct block_device *bdev);