Hi Roger, I have set up a test environment based on Linux 5.11.0-rc4. The patch did not apply clean, so I copied/pasted the patch manually.
Without the patch the call trace (as reported) is visible in dmesg. With the patch the call trace in dmesg is gone, but ... (there is always a but) ... Now the discard action returns the following. [arthur@test-arch ~]$ sudo fstrim -v / fstrim: /: the discard operation is not supported It might be correct, but of course I was hoping the Xen VM guest would pass on the discard request to the block device in the Xen VM host, which is a disk partition. Any suggestions? (Resend due to the previous email being HTML instead of plain text). On Tue, 19 Jan 2021 at 11:57, Roger Pau Monne <[email protected]> wrote: > > This is inline with the specification described in blkif.h: > > * discard-granularity: should be set to the physical block size if > node is not present. > * discard-alignment, discard-secure: should be set to 0 if node not > present. > > This was detected as QEMU would only create the discard-granularity > node but not discard-alignment, and thus the setup done in > blkfront_setup_discard would fail. > > Fix blkfront_setup_discard to not fail on missing nodes, and also fix > blkif_set_queue_limits to set the discard granularity to the physical > block size if none is specified in xenbus. > > Fixes: ed30bf317c5ce ('xen-blkfront: Handle discard requests.') > Reported-by: Arthur Borsboom <[email protected]> > Signed-off-by: Roger Pau Monné <[email protected]> > --- > Cc: Boris Ostrovsky <[email protected]> > Cc: Juergen Gross <[email protected]> > Cc: Stefano Stabellini <[email protected]> > Cc: Konrad Rzeszutek Wilk <[email protected]> > Cc: "Roger Pau Monné" <[email protected]> > Cc: Jens Axboe <[email protected]> > Cc: [email protected] > Cc: [email protected] > Cc: Arthur Borsboom <[email protected]> > --- > Changes since v2: > - Allow all discard-* nodes to be optional. > --- > drivers/block/xen-blkfront.c | 20 +++++++------------- > 1 file changed, 7 insertions(+), 13 deletions(-) > > diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c > index 5265975b3fba..e1c6798889f4 100644 > --- a/drivers/block/xen-blkfront.c > +++ b/drivers/block/xen-blkfront.c > @@ -945,7 +945,8 @@ static void blkif_set_queue_limits(struct blkfront_info > *info) > if (info->feature_discard) { > blk_queue_flag_set(QUEUE_FLAG_DISCARD, rq); > blk_queue_max_discard_sectors(rq, get_capacity(gd)); > - rq->limits.discard_granularity = info->discard_granularity; > + rq->limits.discard_granularity = info->discard_granularity ?: > + info->physical_sector_size; > rq->limits.discard_alignment = info->discard_alignment; > if (info->feature_secdiscard) > blk_queue_flag_set(QUEUE_FLAG_SECERASE, rq); > @@ -2179,19 +2180,12 @@ static void blkfront_closing(struct blkfront_info > *info) > > static void blkfront_setup_discard(struct blkfront_info *info) > { > - int err; > - unsigned int discard_granularity; > - unsigned int discard_alignment; > - > info->feature_discard = 1; > - err = xenbus_gather(XBT_NIL, info->xbdev->otherend, > - "discard-granularity", "%u", &discard_granularity, > - "discard-alignment", "%u", &discard_alignment, > - NULL); > - if (!err) { > - info->discard_granularity = discard_granularity; > - info->discard_alignment = discard_alignment; > - } > + info->discard_granularity = > xenbus_read_unsigned(info->xbdev->otherend, > + > "discard-granularity", > + 0); > + info->discard_alignment = xenbus_read_unsigned(info->xbdev->otherend, > + "discard-alignment", > 0); > info->feature_secdiscard = > !!xenbus_read_unsigned(info->xbdev->otherend, > "discard-secure", > 0); > -- > 2.29.2 > -- Arthur Borsboom
