[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] xen, blkback: fix persistent grants negotiation
From: SeongJae Park <sjpark@xxxxxxxxx> On Thu, 6 Jan 2022 09:10:13 +0000 Maximilian Heyne <mheyne@xxxxxxxxx> wrote: > Given dom0 supports persistent grants but the guest does not. > Then, when attaching a block device during runtime of the guest, dom0 > will enable persistent grants for this newly attached block device: > > $ xenstore-ls -f | grep 20674 | grep persistent > /local/domain/0/backend/vbd/20674/768/feature-persistent = "0" > /local/domain/0/backend/vbd/20674/51792/feature-persistent = "1" > > Here disk 768 was attached during guest creation while 51792 was > attached at runtime. If the guest would have advertised the persistent > grant feature, there would be a xenstore entry like: > > /local/domain/20674/device/vbd/51792/feature-persistent = "1" > > Persistent grants are also used when the guest tries to access the disk > which can be seen when enabling log stats: > > $ echo 1 > /sys/module/xen_blkback/parameters/log_stats > $ dmesg > xen-blkback: (20674.xvdf-0): oo 0 | rd 0 | wr 0 | f 0 | > ds 0 | pg: 1/1056 > > The "pg: 1/1056" shows that one persistent grant is used. > > Before commit aac8a70db24b ("xen-blkback: add a parameter for disabling > of persistent grants") vbd->feature_gnt_persistent was set in > connect_ring. After the commit it was intended to be initialized in > xen_vbd_create and then set according to the guest feature availability > in connect_ring. However, with a running guest, connect_ring might be > called before xen_vbd_create and vbd->feature_gnt_persistent will be > incorrectly initialized. xen_vbd_create will overwrite it with the value > of feature_persistent regardless whether the guest actually supports > persistent grants. > > With this commit, vbd->feature_gnt_persistent is set only in > connect_ring and this is the only use of the module parameter > feature_persistent. This avoids races when the module parameter changes > during the block attachment process. > > Note that vbd->feature_gnt_persistent doesn't need to be initialized in > xen_vbd_create. It's next use is in connect which can only be called > once connect_ring has initialized the rings. xen_update_blkif_status is > checking for this. > > Fixes: aac8a70db24b ("xen-blkback: add a parameter for disabling of > persistent grants") > Signed-off-by: Maximilian Heyne <mheyne@xxxxxxxxx> Thank you for this patch! Reviewed-by: SeongJae Park <sjpark@xxxxxxxxx> Also, I guess this tag is needed? Cc: <stable@xxxxxxxxxxxxxxx> # 5.10.x Thanks, SJ > --- > drivers/block/xen-blkback/xenbus.c | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/block/xen-blkback/xenbus.c > b/drivers/block/xen-blkback/xenbus.c > index 914587aabca0c..51b6ec0380ca4 100644 > --- a/drivers/block/xen-blkback/xenbus.c > +++ b/drivers/block/xen-blkback/xenbus.c > @@ -522,8 +522,6 @@ static int xen_vbd_create(struct xen_blkif *blkif, > blkif_vdev_t handle, > if (q && blk_queue_secure_erase(q)) > vbd->discard_secure = true; > > - vbd->feature_gnt_persistent = feature_persistent; > - > pr_debug("Successful creation of handle=%04x (dom=%u)\n", > handle, blkif->domid); > return 0; > @@ -1090,10 +1088,9 @@ static int connect_ring(struct backend_info *be) > xenbus_dev_fatal(dev, err, "unknown fe protocol %s", protocol); > return -ENOSYS; > } > - if (blkif->vbd.feature_gnt_persistent) > - blkif->vbd.feature_gnt_persistent = > - xenbus_read_unsigned(dev->otherend, > - "feature-persistent", 0); > + > + blkif->vbd.feature_gnt_persistent = feature_persistent && > + xenbus_read_unsigned(dev->otherend, "feature-persistent", 0); > > blkif->vbd.overflow_max_grants = 0; > > -- > 2.32.0 > > > > > Amazon Development Center Germany GmbH > Krausenstr. 38 > 10117 Berlin > Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss > Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B > Sitz: Berlin > Ust-ID: DE 289 237 879 >
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |