Received: by 2002:a25:f815:0:0:0:0:0 with SMTP id u21csp3673991ybd; Tue, 25 Jun 2019 06:40:17 -0700 (PDT) X-Google-Smtp-Source: APXvYqxPFqDz1IsOoLVsy3t/tb+zjT0nshtz4SJTwUEMrmjkEh7XTNv0Ty5I7KlVoeIhjqTPXc4a X-Received: by 2002:a17:902:b591:: with SMTP id a17mr92131855pls.96.1561470017283; Tue, 25 Jun 2019 06:40:17 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1561470017; cv=none; d=google.com; s=arc-20160816; b=l2/BJ5+XoeLDi1ejE1iOFxV4mURObpMc6apTD60YPE4ulnj6a22rtBu06ZZA7vlc35 3XPsdtjUZS/HEQXEC2+4um/IZmVk9xgdPlK0dBnJsLreLXb0umGMwo6NJ32DccWedVBz FmZ+e72GxmSbqo/WwQcfD3HdkFU8XMDCkpJwdUv4zk/Fn3XueMMM3Qc9nHM5iGYFvA+F zXMtoDIACDbjJ3WPm8oCtGvMKS2pI7Br97q48+SzgC6Q7r9raoOcZtd7KhQk+6Hhr1FL p76w75OK1IxBu07IILOqArevc3tNOt0GHrPylp0N2wWWqfy64ttbaYAm/0tUo/2e7PRj oySg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject:dkim-signature; bh=ARK2prxRV/r8YkWxnnbALm6Pw85xxSzYq4sr4VIfIYM=; b=Mjg/rwT1T2Bt+GQSSlQoo1lLJDCxBW9CeXuv8yiBLi+S0Sn52uPkBfuGuS9nSODshh dePhgnUGx4t62mpvJAggiQtZGjLM/wir1NAI9m72q7VoKZbQxrVDV1DFPojW0oL8fI/i JP8Ga7ay7VKfsAFhT00hGm8KK8Fl47VAh+aN0v0xW5bHslgJEFOgTxOiDEblpn/YXdfN iJMVWgK9Pv0VUgfJXhWYFMK+s1zjiFiI3hV5RAzgziu6ixGEpJSwcDY3yP0u3DchPaEA 61l96LLkxzUHw5ml+kLbpru+4tVW1Ut7jhscg1rpi8Kzbs3yEGe4VYFwpdbwvKAgpgyq 4x9A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@lightnvm-io.20150623.gappssmtp.com header.s=20150623 header.b=UkrW7PyH; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id ci23si2832126pjb.85.2019.06.25.06.40.01; Tue, 25 Jun 2019 06:40:17 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@lightnvm-io.20150623.gappssmtp.com header.s=20150623 header.b=UkrW7PyH; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730496AbfFYND6 (ORCPT + 99 others); Tue, 25 Jun 2019 09:03:58 -0400 Received: from mail-lj1-f194.google.com ([209.85.208.194]:42468 "EHLO mail-lj1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728449AbfFYND6 (ORCPT ); Tue, 25 Jun 2019 09:03:58 -0400 Received: by mail-lj1-f194.google.com with SMTP id t28so16163382lje.9 for ; Tue, 25 Jun 2019 06:03:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=lightnvm-io.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=ARK2prxRV/r8YkWxnnbALm6Pw85xxSzYq4sr4VIfIYM=; b=UkrW7PyHLM+UCjb9yNI25LpeVNz39xUL9IGQsHEnSehYh9reiTPFnLo9YPWm1VgUAc 0iCj70AO4HDdbWbKWKBvit4BXpFxpcEmJdZgNglmsuoJpiC10Z+jxs47WMxyb1WCErIb FiKkEPSVJqpgSegB1A3nSFoIwkdDlBue8EQuXk4W+9hdJ6Xd4dofZmW9UCkfHjJXMrjv Rl2mK+FyUsa9j/j3NsMT2RIF/rou5i5LIu3ieZQNbeJR5r4vOIpeVXfdr0Nru4t6hS52 RqELOSABdrgUuyBHfbFUkYMtJk9+xnk7Nd4WkJxmJMxYM+WQ0wlgaGE7cufyyBvINyWJ S9mw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=ARK2prxRV/r8YkWxnnbALm6Pw85xxSzYq4sr4VIfIYM=; b=ZKxnHEQEgTsSGNJUGKZEYaj28L5Qh7MGNTaQKPUUQQ4moev1SSy9G2tlBSbiUx6Pww djAZcgq8djHMOhhmrFT7WuXyL81tW6HeNFOCiPTyIJp35q7MgPKVbqbkN0qdPoyjDQRx SMb4w9DWh14OWYQdsubu6W8bbrbQpZ1jFFhtdrzQWIVPIX0u4PteoqhiNH7zBbUfZUtm IQd1/FnGuHXTrvDwfRDqdcl9raFrv+avaXK9Dx3izozTY3+ErWLOmoEY0A2NP6zr9Lun 7FDAKoZZStQIUaDbFbXsoxG/Ub7I6CwfLeBIyy1PGwvjLZ40yANBxcHa45BrJYUDixRP vixA== X-Gm-Message-State: APjAAAVNZfUq784Dta9GnxmiiClxxnH/R8VLbZpA2RoCm6NFfZAOx6mC zWRTDgIjohEQjiKZf5mDpPGEUA== X-Received: by 2002:a2e:9e1a:: with SMTP id e26mr18452425ljk.158.1561467835635; Tue, 25 Jun 2019 06:03:55 -0700 (PDT) Received: from [192.168.0.36] (2-111-91-225-cable.dk.customer.tdc.net. [2.111.91.225]) by smtp.googlemail.com with ESMTPSA id x22sm1972057lfq.20.2019.06.25.06.03.53 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 25 Jun 2019 06:03:54 -0700 (PDT) Subject: Re: [PATCH 2/4] null_blk: add zone open, close, and finish support To: Damien Le Moal , "axboe@fb.com" , "hch@lst.de" , Chaitanya Kulkarni , Dmitry Fomichev , Ajay Joshi , Aravind Ramesh , "martin.petersen@oracle.com" , "James.Bottomley@HansenPartnership.com" , "agk@redhat.com" , "snitzer@redhat.com" Cc: "linux-block@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-scsi@vger.kernel.org" , "dm-devel@redhat.com" , Matias Bjorling References: <20190621130711.21986-1-mb@lightnvm.io> <20190621130711.21986-3-mb@lightnvm.io> <1aa6552c-ecf9-a168-df75-ec8c52ddbea6@lightnvm.io> From: =?UTF-8?Q?Matias_Bj=c3=b8rling?= Message-ID: Date: Tue, 25 Jun 2019 15:03:53 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 6/25/19 2:36 PM, Damien Le Moal wrote: > On 2019/06/25 20:06, Matias Bjørling wrote: >> On 6/22/19 3:02 AM, Damien Le Moal wrote: >>> On 2019/06/21 22:07, Matias Bjørling wrote: >>>> From: Ajay Joshi >>>> >>>> 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 >>>> Signed-off-by: Matias Bjørling >>>> --- >>>> 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; >>> >> Is this only ZBC? I can't find a reference to it in ZAC. I think it >> should fail. One is trying to open a zone that is full, one can't open >> it again. It's done for this round. > > Page 52/53, section 5.2.6.3.2: > > If the OPEN ALL bit is cleared to zero and the zone specified by the ZONE ID > field (see 5.2.4.3.3) is in Zone Condition: > a) EMPTY, IMPLICITLY OPENED, or CLOSED, then the device shall process an > Explicitly Open Zone function > (see 4.6.3.4.10) for the zone specified by the ZONE ID field; > b) EXPLICITLY OPENED or FULL, then the device shall: > A) not change the zone's state; and > B) return successful command completion; > >>> >>>> + 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. >> >> I think this should cause error. > > See ZAB/ZAC close command description. Same text as above, almost. Not an error. > It is a nop. ZAC page 48, section 5.2.4.3.2: > > If the CLOSE ALL bit is cleared to zero and the zone specified by the ZONE ID > field (see 5.2.4.3.3) is in Zone Condition: > a) IMPLICITLY OPENED, or EXPLICITLY OPENED, then the device shall process a > Close Zone function > (see 4.6.3.4.11) for the zone specified by the ZONE ID field; > b) EMPTY, CLOSED, or FULL, then the device shall: > A) not change the zone's state; and > B) return successful command completion; > >> >> 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. >> >> Only on a reset event right? In general, if I do a expl. open, close it, >> it should not go to empty. > > See the zone state machine. It does return to empty from expl open if nothing > was written, that is, if the WP is still at zone start. This text is in ZAC > section 4.6.3.4.11 as noted above: > > For the specified zone, the Zone Condition state machine processing of this > function (e.g., as shown in the ZC2: Implicit_Open state (see 4.6.3.4.3)) > results in the Zone Condition for the specified zone becoming: > a) EMPTY, if the write pointer indicates the lowest LBA in the zone and Non > Sequential Write Resources Active is false; or > b) CLOSED, if the write pointer does not indicate the lowest LBA in the zone or > Non-Sequential Write Resources Active is true. > Schooled! That is what one gets from having the spec in paper form on the table ;) >> >> 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; >>>> + } >>>> } >>>> >>> >>> >> >> > >