| Message ID | 20200516035434.82809-3-colyli@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block layer change necessary for bcache zoned device support | expand |
On 2020/05/16 12:55, Coly Li wrote: > For a zoned device, e.g. host managed SMR hard drive, the request op > REQ_OP_ZONE_RESET_ALL is to reset LBA of all zones' writer pointers to > the start LBA of the zone they belong to. After all the write pointers > are reset, all previously stored data is invalid and unaccessible. > Therefore this op code changes on-disk data, belongs to WRITE request > op code. > > Similar to the problem of REQ_OP_ZONE_RESET, REQ_OP_ZONE_RESET_ALL is > even number 8, it means bios with REQ_OP_ZONE_RESET_ALL in bio->bi_opf > will be treated as READ request by op_is_write(). > > Same problem will happen when bcache device is created on top of zoned > device like host managed SMR hard drive, bcache driver should invalid > all cached data for the REQ_OP_ZONE_RESET_ALL request, but this zone > management bio is mistakenly treated as READ request and go into bcache > read code path, which will cause undefined behavior. > > This patch changes REQ_OP_ZONE_RESET_ALL value from 8 to 15, this new > odd number will make bcache driver handle this zone management bio in > write request code path properly. > > Fixes: e84e8f066395 ("block: add req op to reset all zones and flag") > Signed-off-by: Coly Li <colyli@suse.de> > Cc: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> > Cc: Damien Le Moal <damien.lemoal@wdc.com> > Cc: Hannes Reinecke <hare@suse.com> > Cc: Jens Axboe <axboe@kernel.dk> > Cc: Johannes Thumshirn <johannes.thumshirn@wdc.com> > Cc: Keith Busch <kbusch@kernel.org> > --- > Changelog: > v2: fix typo for REQ_OP_ZONE_RESET_ALL. > v1: initial version. > > include/linux/blk_types.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h > index 8f7bc15a6de3..618032fa1b29 100644 > --- a/include/linux/blk_types.h > +++ b/include/linux/blk_types.h > @@ -284,8 +284,6 @@ enum req_opf { > REQ_OP_SECURE_ERASE = 5, > /* write the same sector many times */ > REQ_OP_WRITE_SAME = 7, > - /* reset all the zone present on the device */ > - REQ_OP_ZONE_RESET_ALL = 8, > /* write the zero filled sector many times */ > REQ_OP_WRITE_ZEROES = 9, > /* Open a zone */ > @@ -296,6 +294,8 @@ enum req_opf { > REQ_OP_ZONE_FINISH = 12, > /* reset a zone write pointer */ > REQ_OP_ZONE_RESET = 13, > + /* reset all the zone present on the device */ > + REQ_OP_ZONE_RESET_ALL = 15, Same comment as for patch 1. And you can probably squash this patch into patch 1 to avoid repeating mostly the same explanation twice in the commit message. REQ_OP_ZONE_RESET and REQ_OP_ZONE_RESET_ALL are very closely related. The latter may actually be processed using the former anyway, so the op number change for both should be a single patch to be consistent. > > /* SCSI passthrough using struct scsi_request */ > REQ_OP_SCSI_IN = 32, >
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h index 8f7bc15a6de3..618032fa1b29 100644 --- a/include/linux/blk_types.h +++ b/include/linux/blk_types.h @@ -284,8 +284,6 @@ enum req_opf { REQ_OP_SECURE_ERASE = 5, /* write the same sector many times */ REQ_OP_WRITE_SAME = 7, - /* reset all the zone present on the device */ - REQ_OP_ZONE_RESET_ALL = 8, /* write the zero filled sector many times */ REQ_OP_WRITE_ZEROES = 9, /* Open a zone */ @@ -296,6 +294,8 @@ enum req_opf { REQ_OP_ZONE_FINISH = 12, /* reset a zone write pointer */ REQ_OP_ZONE_RESET = 13, + /* reset all the zone present on the device */ + REQ_OP_ZONE_RESET_ALL = 15, /* SCSI passthrough using struct scsi_request */ REQ_OP_SCSI_IN = 32,
For a zoned device, e.g. host managed SMR hard drive, the request op REQ_OP_ZONE_RESET_ALL is to reset LBA of all zones' writer pointers to the start LBA of the zone they belong to. After all the write pointers are reset, all previously stored data is invalid and unaccessible. Therefore this op code changes on-disk data, belongs to WRITE request op code. Similar to the problem of REQ_OP_ZONE_RESET, REQ_OP_ZONE_RESET_ALL is even number 8, it means bios with REQ_OP_ZONE_RESET_ALL in bio->bi_opf will be treated as READ request by op_is_write(). Same problem will happen when bcache device is created on top of zoned device like host managed SMR hard drive, bcache driver should invalid all cached data for the REQ_OP_ZONE_RESET_ALL request, but this zone management bio is mistakenly treated as READ request and go into bcache read code path, which will cause undefined behavior. This patch changes REQ_OP_ZONE_RESET_ALL value from 8 to 15, this new odd number will make bcache driver handle this zone management bio in write request code path properly. Fixes: e84e8f066395 ("block: add req op to reset all zones and flag") Signed-off-by: Coly Li <colyli@suse.de> Cc: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> Cc: Damien Le Moal <damien.lemoal@wdc.com> Cc: Hannes Reinecke <hare@suse.com> Cc: Jens Axboe <axboe@kernel.dk> Cc: Johannes Thumshirn <johannes.thumshirn@wdc.com> Cc: Keith Busch <kbusch@kernel.org> --- Changelog: v2: fix typo for REQ_OP_ZONE_RESET_ALL. v1: initial version. include/linux/blk_types.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)