| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <BYAPR04MB5816D471063D970DDCF9AEC7E7E60@BYAPR04MB5816.namprd04.prod.outlook.com>
Date: Sat, 22 Jun 2019 01:02:57 +0000
From: Damien Le Moal <Damien.LeMoal@....com>
To: Matias Bjørling <mb@...htnvm.io>,
"axboe@...com" <axboe@...com>, "hch@....de" <hch@....de>,
Chaitanya Kulkarni <Chaitanya.Kulkarni@....com>,
Dmitry Fomichev <Dmitry.Fomichev@....com>,
Ajay Joshi <Ajay.Joshi@....com>,
Aravind Ramesh <Aravind.Ramesh@....com>,
"martin.petersen@...cle.com" <martin.petersen@...cle.com>,
"James.Bottomley@...senPartnership.com"
<James.Bottomley@...senPartnership.com>,
"agk@...hat.com" <agk@...hat.com>,
"snitzer@...hat.com" <snitzer@...hat.com>
CC: "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
"dm-devel@...hat.com" <dm-devel@...hat.com>,
Matias Bjorling <Matias.Bjorling@....com>
Subject: Re: [PATCH 2/4] null_blk: add zone open, close, and finish support
On 2019/06/21 22:07, Matias Bjørling wrote:
> From: Ajay Joshi <ajay.joshi@....com>
>
> Implement REQ_OP_ZONE_OPEN, REQ_OP_ZONE_CLOSE and REQ_OP_ZONE_FINISH
> support to allow explicit control of zone states.
>
> Signed-off-by: Ajay Joshi <ajay.joshi@....com>
> Signed-off-by: Matias Bjørling <matias.bjorling@....com>
> ---
> drivers/block/null_blk.h | 4 ++--
> drivers/block/null_blk_main.c | 13 ++++++++++---
> drivers/block/null_blk_zoned.c | 33 ++++++++++++++++++++++++++++++---
> 3 files changed, 42 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/block/null_blk.h b/drivers/block/null_blk.h
> index 34b22d6523ba..62ef65cb0f3e 100644
> --- a/drivers/block/null_blk.h
> +++ b/drivers/block/null_blk.h
> @@ -93,7 +93,7 @@ int null_zone_report(struct gendisk *disk, sector_t sector,
> gfp_t gfp_mask);
> void null_zone_write(struct nullb_cmd *cmd, sector_t sector,
> unsigned int nr_sectors);
> -void null_zone_reset(struct nullb_cmd *cmd, sector_t sector);
> +void null_zone_mgmt_op(struct nullb_cmd *cmd, sector_t sector);
> #else
> static inline int null_zone_init(struct nullb_device *dev)
> {
> @@ -111,6 +111,6 @@ static inline void null_zone_write(struct nullb_cmd *cmd, sector_t sector,
> unsigned int nr_sectors)
> {
> }
> -static inline void null_zone_reset(struct nullb_cmd *cmd, sector_t sector) {}
> +static inline void null_zone_mgmt_op(struct nullb_cmd *cmd, sector_t sector) {}
> #endif /* CONFIG_BLK_DEV_ZONED */
> #endif /* __NULL_BLK_H */
> diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
> index 447d635c79a2..5058fb980c9c 100644
> --- a/drivers/block/null_blk_main.c
> +++ b/drivers/block/null_blk_main.c
> @@ -1209,10 +1209,17 @@ static blk_status_t null_handle_cmd(struct nullb_cmd *cmd)
> nr_sectors = blk_rq_sectors(cmd->rq);
> }
>
> - if (op == REQ_OP_WRITE)
> + switch (op) {
> + case REQ_OP_WRITE:
> null_zone_write(cmd, sector, nr_sectors);
> - else if (op == REQ_OP_ZONE_RESET)
> - null_zone_reset(cmd, sector);
> + break;
> + case REQ_OP_ZONE_RESET:
> + case REQ_OP_ZONE_OPEN:
> + case REQ_OP_ZONE_CLOSE:
> + case REQ_OP_ZONE_FINISH:
> + null_zone_mgmt_op(cmd, sector);
> + break;
> + }
> }
> out:
> /* Complete IO by inline, softirq or timer */
> diff --git a/drivers/block/null_blk_zoned.c b/drivers/block/null_blk_zoned.c
> index fca0c97ff1aa..47d956b2e148 100644
> --- a/drivers/block/null_blk_zoned.c
> +++ b/drivers/block/null_blk_zoned.c
> @@ -121,17 +121,44 @@ void null_zone_write(struct nullb_cmd *cmd, sector_t sector,
> }
> }
>
> -void null_zone_reset(struct nullb_cmd *cmd, sector_t sector)
> +void null_zone_mgmt_op(struct nullb_cmd *cmd, sector_t sector)
> {
> struct nullb_device *dev = cmd->nq->dev;
> unsigned int zno = null_zone_no(dev, sector);
> struct blk_zone *zone = &dev->zones[zno];
> + enum req_opf op = req_op(cmd->rq);
>
> if (zone->type == BLK_ZONE_TYPE_CONVENTIONAL) {
> cmd->error = BLK_STS_IOERR;
> return;
> }
>
> - zone->cond = BLK_ZONE_COND_EMPTY;
> - zone->wp = zone->start;
> + switch (op) {
> + case REQ_OP_ZONE_RESET:
> + zone->cond = BLK_ZONE_COND_EMPTY;
> + zone->wp = zone->start;
> + return;
> + case REQ_OP_ZONE_OPEN:
> + if (zone->cond == BLK_ZONE_COND_FULL) {
> + cmd->error = BLK_STS_IOERR;
> + return;
> + }
> + zone->cond = BLK_ZONE_COND_EXP_OPEN;
With ZBC, open of a full zone is a "nop". No error. So I would rather have this as:
if (zone->cond != BLK_ZONE_COND_FULL)
zone->cond = BLK_ZONE_COND_EXP_OPEN;
> + return;
> + case REQ_OP_ZONE_CLOSE:
> + if (zone->cond == BLK_ZONE_COND_FULL) {
> + cmd->error = BLK_STS_IOERR;
> + return;
> + }
> + zone->cond = BLK_ZONE_COND_CLOSED;
Sam as for open. Closing a full zone on ZBC is a nop. And the code above would
also set an empty zone to closed. Finally, if the zone is open but nothing was
written to it, it must be returned to empty condition, not closed. So something
like this is needed.
switch (zone->cond) {
case BLK_ZONE_COND_FULL:
case BLK_ZONE_COND_EMPTY:
break;
case BLK_ZONE_COND_EXP_OPEN:
if (zone->wp == zone->start) {
zone->cond = BLK_ZONE_COND_EMPTY;
break;
}
/* fallthrough */
default:
zone->cond = BLK_ZONE_COND_CLOSED;
}
> + return;
> + case REQ_OP_ZONE_FINISH:
> + zone->cond = BLK_ZONE_COND_FULL;
> + zone->wp = zone->start + zone->len;
> + return;
> + default:
> + /* Invalid zone condition */
> + cmd->error = BLK_STS_IOERR;
> + return;
> + }
> }
>
--
Damien Le Moal
Western Digital Research
Powered by blists - more mailing lists