diff mbox series

[RFC,v2,for-6.8/block,01/18] block: add some bdev apis

Message ID 20231211140552.973290-2-yukuai1@huaweicloud.com (mailing list archive)
State New, archived
Headers show
Series block: don't access bd_inode directly from other modules | expand

Commit Message

Yu Kuai Dec. 11, 2023, 2:05 p.m. UTC
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>
---
 block/bdev.c           | 70 ++++++++++++++++++++++++++++++++++++++++++
 block/blk.h            |  2 --
 include/linux/blkdev.h | 17 ++++++++++
 3 files changed, 87 insertions(+), 2 deletions(-)

Comments

Jan Kara Dec. 11, 2023, 4:52 p.m. UTC | #1
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
Yu Kuai Dec. 12, 2023, 1:25 a.m. UTC | #2
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
>
Christoph Hellwig Dec. 12, 2023, 1:14 p.m. UTC | #3
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.
Christoph Hellwig Dec. 12, 2023, 1:16 p.m. UTC | #4
> +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.
Yu Kuai Dec. 13, 2023, 1:09 a.m. UTC | #5
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
> 
> .
>
Yu Kuai Dec. 13, 2023, 1:10 a.m. UTC | #6
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 mbox series

Patch

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);