| Message ID | 20220211093425.43262-2-p.raghav@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Regression in ZNS devices due to batched completions | expand |
On 11/02/2022 10:42, Pankaj Raghav wrote: > Zone append command needs special handling to update the bi_sector > field in the bio struct with the actual position of the data in the > device. It is stored in __sector field of the request struct. > > Fixes: 5581a5ddfe8d ("block: add completion handler for fast path") > Signed-off-by: Pankaj Raghav <p.raghav@samsung.com> I would've preferred if you had put the trace and steps to reproduce in the changelog instead of the cover letter but, maybe Jens can update it when he applies the patch. Anyways, Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
On Fri, Feb 11, 2022 at 10:02:26AM +0000, Johannes Thumshirn wrote: > On 11/02/2022 10:42, Pankaj Raghav wrote: > > Zone append command needs special handling to update the bi_sector > > field in the bio struct with the actual position of the data in the > > device. It is stored in __sector field of the request struct. > > > > Fixes: 5581a5ddfe8d ("block: add completion handler for fast path") > > Signed-off-by: Pankaj Raghav <p.raghav@samsung.com> > > I would've preferred if you had put the trace and steps to reproduce > in the changelog instead of the cover letter but, maybe Jens can update > it when he applies the patch. That would be wonderful. And a new fstests :) I'm surprised no one caught this earlier... does this mean... no fstests yet covers this? If a test does... then does that mean... No one is testing zone btrfs... since... around December (5581a5ddfe8d) or January (merge commit to Linus)? > Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> Luis
On Fri, Feb 11, 2022 at 10:34:25AM +0100, Pankaj Raghav wrote: > Zone append command needs special handling to update the bi_sector > field in the bio struct with the actual position of the data in the > device. It is stored in __sector field of the request struct. > > Fixes: 5581a5ddfe8d ("block: add completion handler for fast path") > Signed-off-by: Pankaj Raghav <p.raghav@samsung.com> > --- > block/blk-mq.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/block/blk-mq.c b/block/blk-mq.c > index 4b868e792ba4..6c2231e52991 100644 > --- a/block/blk-mq.c > +++ b/block/blk-mq.c > @@ -736,6 +736,10 @@ static void blk_complete_request(struct request *req) > > /* Completion has already been traced */ > bio_clear_flag(bio, BIO_TRACE_COMPLETION); > + > + if (req_op(req) == REQ_OP_ZONE_APPEND) > + bio->bi_iter.bi_sector = req->__sector; > + > if (!is_flush) > bio_endio(bio); > bio = next; Nice catch and thanks for including steps to reproduce. Tested-by: Adam Manzanares <a.manzanares@samsung.com> > -- > 2.25.1 >
On 11/02/2022 17:17, Luis Chamberlain wrote: > On Fri, Feb 11, 2022 at 10:02:26AM +0000, Johannes Thumshirn wrote: >> On 11/02/2022 10:42, Pankaj Raghav wrote: >>> Zone append command needs special handling to update the bi_sector >>> field in the bio struct with the actual position of the data in the >>> device. It is stored in __sector field of the request struct. >>> >>> Fixes: 5581a5ddfe8d ("block: add completion handler for fast path") >>> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com> >> >> I would've preferred if you had put the trace and steps to reproduce >> in the changelog instead of the cover letter but, maybe Jens can update >> it when he applies the patch. > > That would be wonderful. And a new fstests :) > > I'm surprised no one caught this earlier... does this mean... no fstests > yet covers this? If a test does... then does that mean... > > No one is testing zone btrfs... since... around December > (5581a5ddfe8d) or January (merge commit to Linus)? Ahm no. Please be sure we do test zoned btrfs. But yes, I'm at the moment not testing on ZNS drives, so I haven't caught the bug earlier.
diff --git a/block/blk-mq.c b/block/blk-mq.c index 4b868e792ba4..6c2231e52991 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -736,6 +736,10 @@ static void blk_complete_request(struct request *req) /* Completion has already been traced */ bio_clear_flag(bio, BIO_TRACE_COMPLETION); + + if (req_op(req) == REQ_OP_ZONE_APPEND) + bio->bi_iter.bi_sector = req->__sector; + if (!is_flush) bio_endio(bio); bio = next;
Zone append command needs special handling to update the bi_sector field in the bio struct with the actual position of the data in the device. It is stored in __sector field of the request struct. Fixes: 5581a5ddfe8d ("block: add completion handler for fast path") Signed-off-by: Pankaj Raghav <p.raghav@samsung.com> --- block/blk-mq.c | 4 ++++ 1 file changed, 4 insertions(+)