* [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
` (4 subsequent siblings)
5 siblings, 1 reply; 10+ 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 related [flat|nested] 10+ 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
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ 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 related [flat|nested] 10+ 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
` (2 subsequent siblings)
5 siblings, 1 reply; 10+ 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 related [flat|nested] 10+ 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; 10+ 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] 10+ 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
2021-10-21 3:41 ` [PATCH v2 0/4] Add invalidate_disk() helper for drivers to invalidate the gendisk Yongji Xie
2021-10-21 16:12 ` Jens Axboe
5 siblings, 1 reply; 10+ 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 related [flat|nested] 10+ 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; 10+ 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] 10+ messages in thread
* Re: [PATCH v2 0/4] Add invalidate_disk() helper for drivers 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
` (3 preceding siblings ...)
2021-09-22 12:37 ` [PATCH v2 4/4] nbd: Use invalidate_disk() helper on disconnect Xie Yongji
@ 2021-10-21 3:41 ` Yongji Xie
2021-10-21 16:12 ` Jens Axboe
5 siblings, 0 replies; 10+ messages in thread
From: Yongji Xie @ 2021-10-21 3:41 UTC (permalink / raw)
To: Jens Axboe, Josef Bacik, Christoph Hellwig; +Cc: linux-block, nbd, yixingchen
Ping
On Wed, Sep 22, 2021 at 8:37 PM Xie Yongji <xieyongji@bytedance.com> wrote:
>
> 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] 10+ messages in thread* Re: [PATCH v2 0/4] Add invalidate_disk() helper for drivers 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
` (4 preceding siblings ...)
2021-10-21 3:41 ` [PATCH v2 0/4] Add invalidate_disk() helper for drivers to invalidate the gendisk Yongji Xie
@ 2021-10-21 16:12 ` Jens Axboe
5 siblings, 0 replies; 10+ messages in thread
From: Jens Axboe @ 2021-10-21 16:12 UTC (permalink / raw)
To: hch, josef, Xie Yongji; +Cc: yixingchen, linux-block, nbd
On Wed, 22 Sep 2021 20:37:07 +0800, Xie Yongji wrote:
> 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.
>
> [...]
Applied, thanks!
[1/4] block: Add invalidate_disk() helper to invalidate the gendisk
commit: f059a1d2e23a165bf86e33673c6a7535a08c6341
[2/4] loop: Use invalidate_disk() helper to invalidate gendisk
commit: e515be8f3b3e63be4c5e91dc6620483ed0990a0c
[3/4] loop: Remove the unnecessary bdev checks and unused bdev variable
commit: 19f553db2ac03cb8407ec8efb8e140951afdfb87
[4/4] nbd: Use invalidate_disk() helper on disconnect
commit: 435c2acb307f19acc791b4295e29cc53a82bd24d
Best regards,
--
Jens Axboe
^ permalink raw reply [flat|nested] 10+ messages in thread