linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] Add invalidate_disk() helper for drivers to invalidate the gendisk
@ 2021-09-22 12:37 Xie Yongji
  2021-09-22 12:37 ` [PATCH v2 1/4] block: Add invalidate_disk() helper " Xie Yongji
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Xie Yongji @ 2021-09-22 12:37 UTC (permalink / raw)
  To: axboe, josef, hch; +Cc: linux-block, nbd, yixingchen

This series comes from Christoph Hellwig's suggestion [1]. Some block
device drivers such as loop driver and nbd driver need to invalidate
the gendisk when the backend is detached so that the gendisk can be
reused by the new backend. Now the invalidation is done in device
driver with their own ways. To avoid code duplication and hide
some internals of the implementation, this series adds a block layer
helper and makes both loop driver and nbd driver use it.

[1] https://lore.kernel.org/all/YTmqJHd7YWAQ2lZ7@infradead.org/

V1 to V2:
- Rename invalidate_gendisk() to invalidate_disk()
- Add a cleanup patch to remove bdev checks and bdev variable in __loop_clr_fd()

Xie Yongji (4):
  block: Add invalidate_disk() helper to invalidate the gendisk
  loop: Use invalidate_disk() helper to invalidate gendisk
  loop: Remove the unnecessary bdev checks and unused bdev variable
  nbd: Use invalidate_disk() helper on disconnect

 block/genhd.c         | 20 ++++++++++++++++++++
 drivers/block/loop.c  | 15 ++++-----------
 drivers/block/nbd.c   | 12 +++---------
 include/linux/genhd.h |  2 ++
 4 files changed, 29 insertions(+), 20 deletions(-)

-- 
2.11.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/4] block: Add invalidate_disk() helper to invalidate the gendisk
  2021-09-22 12:37 [PATCH v2 0/4] Add invalidate_disk() helper for drivers to invalidate the gendisk Xie Yongji
@ 2021-09-22 12:37 ` Xie Yongji
  2021-09-22 14:41   ` Christoph Hellwig
  2021-09-22 12:37 ` [PATCH v2 2/4] loop: Use invalidate_disk() helper to invalidate gendisk Xie Yongji
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Xie Yongji @ 2021-09-22 12:37 UTC (permalink / raw)
  To: axboe, josef, hch; +Cc: linux-block, nbd, yixingchen

To hide internal implementation and simplify some driver code,
this adds a helper to invalidate the gendisk. It will clean the
gendisk's associated buffer/page caches and reset its internal
states.

Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
 block/genhd.c         | 20 ++++++++++++++++++++
 include/linux/genhd.h |  2 ++
 2 files changed, 22 insertions(+)

diff --git a/block/genhd.c b/block/genhd.c
index 7b6e5e1cf956..876c0e478baf 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -601,6 +601,26 @@ void del_gendisk(struct gendisk *disk)
 }
 EXPORT_SYMBOL(del_gendisk);
 
+/**
+ * invalidate_disk - invalidate the disk
+ * @disk: the struct gendisk to invalidate
+ *
+ * A helper to invalidates the disk. It will clean the disk's associated
+ * buffer/page caches and reset its internal states so that the disk
+ * can be reused by the drivers.
+ *
+ * Context: can sleep
+ */
+void invalidate_disk(struct gendisk *disk)
+{
+	struct block_device *bdev = disk->part0;
+
+	invalidate_bdev(bdev);
+	bdev->bd_inode->i_mapping->wb_err = 0;
+	set_capacity(disk, 0);
+}
+EXPORT_SYMBOL(invalidate_disk);
+
 /* sysfs access to bad-blocks list. */
 static ssize_t disk_badblocks_show(struct device *dev,
 					struct device_attribute *attr,
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index c68d83c87f83..ea815feb3626 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -222,6 +222,8 @@ static inline int add_disk(struct gendisk *disk)
 }
 extern void del_gendisk(struct gendisk *gp);
 
+void invalidate_disk(struct gendisk *disk);
+
 void set_disk_ro(struct gendisk *disk, bool read_only);
 
 static inline int get_disk_ro(struct gendisk *disk)
-- 
2.11.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 2/4] loop: Use invalidate_disk() helper to invalidate gendisk
  2021-09-22 12:37 [PATCH v2 0/4] Add invalidate_disk() helper for drivers to invalidate the gendisk Xie Yongji
  2021-09-22 12:37 ` [PATCH v2 1/4] block: Add invalidate_disk() helper " Xie Yongji
@ 2021-09-22 12:37 ` Xie Yongji
  2021-09-22 12:37 ` [PATCH v2 3/4] loop: Remove the unnecessary bdev checks and unused bdev variable Xie Yongji
  2021-09-22 12:37 ` [PATCH v2 4/4] nbd: Use invalidate_disk() helper on disconnect Xie Yongji
  3 siblings, 0 replies; 8+ messages in thread
From: Xie Yongji @ 2021-09-22 12:37 UTC (permalink / raw)
  To: axboe, josef, hch; +Cc: linux-block, nbd, yixingchen

Use invalidate_disk() helper to simplify the code for gendisk
invalidation.

Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/loop.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 7bf4686af774..eab6906326cc 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1395,11 +1395,7 @@ static int __loop_clr_fd(struct loop_device *lo, bool release)
 	blk_queue_logical_block_size(lo->lo_queue, 512);
 	blk_queue_physical_block_size(lo->lo_queue, 512);
 	blk_queue_io_min(lo->lo_queue, 512);
-	if (bdev) {
-		invalidate_bdev(bdev);
-		bdev->bd_inode->i_mapping->wb_err = 0;
-	}
-	set_capacity(lo->lo_disk, 0);
+	invalidate_disk(lo->lo_disk);
 	loop_sysfs_exit(lo);
 	if (bdev) {
 		/* let user-space know about this change */
-- 
2.11.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 3/4] loop: Remove the unnecessary bdev checks and unused bdev variable
  2021-09-22 12:37 [PATCH v2 0/4] Add invalidate_disk() helper for drivers to invalidate the gendisk Xie Yongji
  2021-09-22 12:37 ` [PATCH v2 1/4] block: Add invalidate_disk() helper " Xie Yongji
  2021-09-22 12:37 ` [PATCH v2 2/4] loop: Use invalidate_disk() helper to invalidate gendisk Xie Yongji
@ 2021-09-22 12:37 ` Xie Yongji
  2021-09-22 14:43   ` Christoph Hellwig
  2021-09-22 12:37 ` [PATCH v2 4/4] nbd: Use invalidate_disk() helper on disconnect Xie Yongji
  3 siblings, 1 reply; 8+ messages in thread
From: Xie Yongji @ 2021-09-22 12:37 UTC (permalink / raw)
  To: axboe, josef, hch; +Cc: linux-block, nbd, yixingchen

The lo->lo_device can't be null if the lo->lo_backing_file is set.
So let's remove the unnecessary bdev checks and the entire bdev
variable in __loop_clr_fd() since the lo->lo_backing_file is already
checked before.

Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
 drivers/block/loop.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index eab6906326cc..980b538c008a 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1329,7 +1329,6 @@ static int __loop_clr_fd(struct loop_device *lo, bool release)
 {
 	struct file *filp = NULL;
 	gfp_t gfp = lo->old_gfp_mask;
-	struct block_device *bdev = lo->lo_device;
 	int err = 0;
 	bool partscan = false;
 	int lo_number;
@@ -1397,16 +1396,14 @@ static int __loop_clr_fd(struct loop_device *lo, bool release)
 	blk_queue_io_min(lo->lo_queue, 512);
 	invalidate_disk(lo->lo_disk);
 	loop_sysfs_exit(lo);
-	if (bdev) {
-		/* let user-space know about this change */
-		kobject_uevent(&disk_to_dev(bdev->bd_disk)->kobj, KOBJ_CHANGE);
-	}
+	/* let user-space know about this change */
+	kobject_uevent(&disk_to_dev(lo->lo_disk)->kobj, KOBJ_CHANGE);
 	mapping_set_gfp_mask(filp->f_mapping, gfp);
 	/* This is safe: open() is still holding a reference. */
 	module_put(THIS_MODULE);
 	blk_mq_unfreeze_queue(lo->lo_queue);
 
-	partscan = lo->lo_flags & LO_FLAGS_PARTSCAN && bdev;
+	partscan = lo->lo_flags & LO_FLAGS_PARTSCAN;
 	lo_number = lo->lo_number;
 	disk_force_media_change(lo->lo_disk, DISK_EVENT_MEDIA_CHANGE);
 out_unlock:
-- 
2.11.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 4/4] nbd: Use invalidate_disk() helper on disconnect
  2021-09-22 12:37 [PATCH v2 0/4] Add invalidate_disk() helper for drivers to invalidate the gendisk Xie Yongji
                   ` (2 preceding siblings ...)
  2021-09-22 12:37 ` [PATCH v2 3/4] loop: Remove the unnecessary bdev checks and unused bdev variable Xie Yongji
@ 2021-09-22 12:37 ` Xie Yongji
  2021-09-22 14:47   ` Christoph Hellwig
  3 siblings, 1 reply; 8+ messages in thread
From: Xie Yongji @ 2021-09-22 12:37 UTC (permalink / raw)
  To: axboe, josef, hch; +Cc: linux-block, nbd, yixingchen

When a nbd device encounters a writeback error, that error will
get propagated to the bd_inode's wb_err field. Then if this nbd
device's backend is disconnected and another is attached, we will
get back the previous writeback error on fsync, which is unexpected.

To fix it, let's use invalidate_disk() helper to invalidate the
disk on disconnect instead of just setting disk's capacity to zero.

Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
 drivers/block/nbd.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 5170a630778d..19f2e1247416 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -305,14 +305,6 @@ static void nbd_mark_nsock_dead(struct nbd_device *nbd, struct nbd_sock *nsock,
 	nsock->sent = 0;
 }
 
-static void nbd_size_clear(struct nbd_device *nbd)
-{
-	if (nbd->config->bytesize) {
-		set_capacity(nbd->disk, 0);
-		kobject_uevent(&nbd_to_dev(nbd)->kobj, KOBJ_CHANGE);
-	}
-}
-
 static int nbd_set_size(struct nbd_device *nbd, loff_t bytesize,
 		loff_t blksize)
 {
@@ -1232,7 +1224,9 @@ static void nbd_config_put(struct nbd_device *nbd)
 					&nbd->config_lock)) {
 		struct nbd_config *config = nbd->config;
 		nbd_dev_dbg_close(nbd);
-		nbd_size_clear(nbd);
+		invalidate_disk(nbd->disk);
+		if (nbd->config->bytesize)
+			kobject_uevent(&nbd_to_dev(nbd)->kobj, KOBJ_CHANGE);
 		if (test_and_clear_bit(NBD_RT_HAS_PID_FILE,
 				       &config->runtime_flags))
 			device_remove_file(disk_to_dev(nbd->disk), &pid_attr);
-- 
2.11.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/4] block: Add invalidate_disk() helper to invalidate the gendisk
  2021-09-22 12:37 ` [PATCH v2 1/4] block: Add invalidate_disk() helper " Xie Yongji
@ 2021-09-22 14:41   ` Christoph Hellwig
  0 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2021-09-22 14:41 UTC (permalink / raw)
  To: Xie Yongji; +Cc: axboe, josef, hch, linux-block, nbd, yixingchen

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 3/4] loop: Remove the unnecessary bdev checks and unused bdev variable
  2021-09-22 12:37 ` [PATCH v2 3/4] loop: Remove the unnecessary bdev checks and unused bdev variable Xie Yongji
@ 2021-09-22 14:43   ` Christoph Hellwig
  0 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2021-09-22 14:43 UTC (permalink / raw)
  To: Xie Yongji; +Cc: axboe, josef, hch, linux-block, nbd, yixingchen

On Wed, Sep 22, 2021 at 08:37:10PM +0800, Xie Yongji wrote:
> The lo->lo_device can't be null if the lo->lo_backing_file is set.
> So let's remove the unnecessary bdev checks and the entire bdev
> variable in __loop_clr_fd() since the lo->lo_backing_file is already
> checked before.
> 
> Signed-off-by: Xie Yongji <xieyongji@bytedance.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 4/4] nbd: Use invalidate_disk() helper on disconnect
  2021-09-22 12:37 ` [PATCH v2 4/4] nbd: Use invalidate_disk() helper on disconnect Xie Yongji
@ 2021-09-22 14:47   ` Christoph Hellwig
  0 siblings, 0 replies; 8+ messages in thread
From: Christoph Hellwig @ 2021-09-22 14:47 UTC (permalink / raw)
  To: Xie Yongji; +Cc: axboe, josef, hch, linux-block, nbd, yixingchen

On Wed, Sep 22, 2021 at 08:37:11PM +0800, Xie Yongji wrote:
> When a nbd device encounters a writeback error, that error will
> get propagated to the bd_inode's wb_err field. Then if this nbd
> device's backend is disconnected and another is attached, we will
> get back the previous writeback error on fsync, which is unexpected.
> 
> To fix it, let's use invalidate_disk() helper to invalidate the
> disk on disconnect instead of just setting disk's capacity to zero.

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-09-22 14:49 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-22 12:37 [PATCH v2 0/4] Add invalidate_disk() helper for drivers to invalidate the gendisk Xie Yongji
2021-09-22 12:37 ` [PATCH v2 1/4] block: Add invalidate_disk() helper " Xie Yongji
2021-09-22 14:41   ` Christoph Hellwig
2021-09-22 12:37 ` [PATCH v2 2/4] loop: Use invalidate_disk() helper to invalidate gendisk Xie Yongji
2021-09-22 12:37 ` [PATCH v2 3/4] loop: Remove the unnecessary bdev checks and unused bdev variable Xie Yongji
2021-09-22 14:43   ` Christoph Hellwig
2021-09-22 12:37 ` [PATCH v2 4/4] nbd: Use invalidate_disk() helper on disconnect Xie Yongji
2021-09-22 14:47   ` Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).