| Message ID | 20200102075315.22652-1-sblbir@amazon.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add support for block disk resize notification | expand |
On 1/2/20 3:53 PM, Balbir Singh wrote: > Allow block/genhd to notify user space about disk size changes > using a new helper disk_set_capacity(), which is a wrapper on top > of set_capacity(). disk_set_capacity() will only notify if > the current capacity or the target capacity is not zero. > set_capacity_and_notify() may be a more straightforward name. > Background: > > As a part of a patch to allow sending the RESIZE event on disk capacity > change, Christoph (hch@lst.de) requested that the patch be made generic > and the hacks for virtio block and xen block devices be removed and > merged via a generic helper. > > This series consists of 5 changes. The first one adds the basic > support for changing the size and notifying. The follow up patches > are per block subsystem changes. Other block drivers can add their > changes as necessary on top of this series. > > Testing: > 1. I did some basic testing with an NVME device, by resizing it in > the backend and ensured that udevd received the event. > > NOTE: After these changes, the notification might happen before > revalidate disk, where as it occured later before. > It's better not to change original behavior. How about something like: +void set_capacity_and_notify(struct gendisk *disk, sector_t size, bool revalidate) { sector_t capacity = get_capacity(disk); set_capacity(disk, size); + if (revalidate) + revalidate_disk(disk); if (capacity != 0 && size != 0) { char *envp[] = { "RESIZE=1", NULL }; kobject_uevent_env(&disk_to_dev(disk)->kobj, KOBJ_CHANGE, envp); } } > Balbir Singh (5): > block/genhd: Notify udev about capacity change > drivers/block/virtio_blk.c: Convert to use disk_set_capacity > drivers/block/xen-blkfront.c: Convert to use disk_set_capacity > drivers/nvme/host/core.c: Convert to use disk_set_capacity > drivers/scsi/sd.c: Convert to use disk_set_capacity > > block/genhd.c | 19 +++++++++++++++++++ > drivers/block/virtio_blk.c | 4 +--- > drivers/block/xen-blkfront.c | 5 +---- > drivers/nvme/host/core.c | 2 +- > drivers/scsi/sd.c | 2 +- > include/linux/genhd.h | 1 + > 6 files changed, 24 insertions(+), 9 deletions(-) >
On Mon, 2020-01-06 at 13:59 +0800, Bob Liu wrote: > On 1/2/20 3:53 PM, Balbir Singh wrote: > > Allow block/genhd to notify user space about disk size changes > > using a new helper disk_set_capacity(), which is a wrapper on top > > of set_capacity(). disk_set_capacity() will only notify if > > the current capacity or the target capacity is not zero. > > > > set_capacity_and_notify() may be a more straightforward name. > Yes, agreed. > > Background: > > > > As a part of a patch to allow sending the RESIZE event on disk capacity > > change, Christoph (hch@lst.de) requested that the patch be made generic > > and the hacks for virtio block and xen block devices be removed and > > merged via a generic helper. > > > > This series consists of 5 changes. The first one adds the basic > > support for changing the size and notifying. The follow up patches > > are per block subsystem changes. Other block drivers can add their > > changes as necessary on top of this series. > > > > Testing: > > 1. I did some basic testing with an NVME device, by resizing it in > > the backend and ensured that udevd received the event. > > > > NOTE: After these changes, the notification might happen before > > revalidate disk, where as it occured later before. > > > > It's better not to change original behavior. > How about something like: > > +void set_capacity_and_notify(struct gendisk *disk, sector_t size, bool > revalidate) > { > sector_t capacity = get_capacity(disk); > > set_capacity(disk, size); > > + if (revalidate) > + revalidate_disk(disk); Do you see a concern with the notification going out before revalidate_disk? I could keep the behaviour and come up with a suitable name revalidate_disk_and_notify() (set_capacity is a part of the revalidation process), or feel free to suggest a better name Thanks, Balbir Singh
On 1/6/20 4:47 PM, Singh, Balbir wrote: > On Mon, 2020-01-06 at 13:59 +0800, Bob Liu wrote: >> On 1/2/20 3:53 PM, Balbir Singh wrote: >>> Allow block/genhd to notify user space about disk size changes >>> using a new helper disk_set_capacity(), which is a wrapper on top >>> of set_capacity(). disk_set_capacity() will only notify if >>> the current capacity or the target capacity is not zero. >>> >> >> set_capacity_and_notify() may be a more straightforward name. >> > > Yes, agreed. > >>> Background: >>> >>> As a part of a patch to allow sending the RESIZE event on disk capacity >>> change, Christoph (hch@lst.de) requested that the patch be made generic >>> and the hacks for virtio block and xen block devices be removed and >>> merged via a generic helper. >>> >>> This series consists of 5 changes. The first one adds the basic >>> support for changing the size and notifying. The follow up patches >>> are per block subsystem changes. Other block drivers can add their >>> changes as necessary on top of this series. >>> >>> Testing: >>> 1. I did some basic testing with an NVME device, by resizing it in >>> the backend and ensured that udevd received the event. >>> >>> NOTE: After these changes, the notification might happen before >>> revalidate disk, where as it occured later before. >>> >> >> It's better not to change original behavior. >> How about something like: >> >> +void set_capacity_and_notify(struct gendisk *disk, sector_t size, bool >> revalidate) >> { >> sector_t capacity = get_capacity(disk); >> >> set_capacity(disk, size); >> >> + if (revalidate) >> + revalidate_disk(disk); > > Do you see a concern with the notification going out before revalidate_disk? Not aware any, but keep the original behavior is safer especial when not must change.. > I could keep the behaviour and come up with a suitable name > > revalidate_disk_and_notify() (set_capacity is a part of the revalidation > process), or feel free to suggest a better name > Feel free to add my reviewed-by next version in this series. Reviewed-by: Bob Liu <bob.liu@oracle.com> > Thanks, > Balbir Singh >