Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: ZGJPNbtI3I2Myj6BkEhxhGLDMgTOVzld9HMMjz06J4yWtdeSn3SmhVHBuh6miHu8BjvQcmu2jJ
 kTZkqa4RsYWkeIkMjxJoSKGo4IgM3p19v0eje1guJ24GAg++/WgcjgL5mrE6AydUsW7CWRzwQf
 P7WG+ijls8D+bAOux8w9FoqoQTLsmKrkR+eoiVhRU8uuEGg8TYyo3X2oQms9wLPYBo0/cOrWGw
 f72/x+Zd7U5uHD18PutFweaU2WITGDUJeh269L6U3ViNMkVh1mgSdXERAoHRM2cos9bZvHQGCO
 Y5o=
From:   Damien Le Moal <Damien.LeMoal@wdc.com>
To:     Niklas Cassel <Niklas.Cassel@wdc.com>
CC:     Jens Axboe <axboe@kernel.dk>,
        Johannes Thumshirn <Johannes.Thumshirn@wdc.com>,
        "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] null_blk: add support for max open/active zone limit
 for zoned devices
Thread-Topic: [PATCH v2] null_blk: add support for max open/active zone limit
 for zoned devices
Thread-Index: AQHWfHkH+20N/yg19E+BQ9ptEBewKg==
Date:   Fri, 28 Aug 2020 10:29:17 +0000
Message-ID: <CY4PR04MB375160276B09E980D1B1158AE7520@CY4PR04MB3751.namprd04.prod.outlook.com>
References: <20200827135018.63644-1-niklas.cassel@wdc.com>
 <CY4PR04MB3751526DC5833902749EBE48E7520@CY4PR04MB3751.namprd04.prod.outlook.com>
 <20200828100655.GA71979@localhost.localdomain>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 0fd13f0d-85a3-4d5a-e645-08d84b3d382c
X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Aug 2020 10:29:17.9050
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 1n+Xvy3wSeVtSbuRxVbh1sAIFKSFEQdZZ9CW4Parf5yIHn1xkYO7akIb2s4zML3FV6U1+QNlMzKf1dMIlJRJVg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR0401MB3633
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2020/08/28 19:06, Niklas Cassel wrote:=0A=
> On Fri, Aug 28, 2020 at 07:06:26AM +0000, Damien Le Moal wrote:=0A=
>> On 2020/08/27 22:50, Niklas Cassel wrote:=0A=
>>> Add support for user space to set a max open zone and a max active zone=
=0A=
>>> limit via configfs. By default, the default values are 0 =3D=3D no limi=
t.=0A=
>>>=0A=
>>> Call the block layer API functions used for exposing the configured=0A=
>>> limits to sysfs.=0A=
>>>=0A=
>>> Add accounting in null_blk_zoned so that these new limits are respected=
.=0A=
>>> Performing an operating that would exceed these limits results in a=0A=
>>=0A=
>> Performing a write operation that would result in exceeding these...=0A=
>>=0A=
>>> standard I/O error.=0A=
>>>=0A=
> =0A=
> It is not only a write operation, also e.g. open zone operation.=0A=
> However I will s/Performing an operating/Performing an operation/=0A=
> =0A=
>>> +/*=0A=
>>> + * This function matches the manage open zone resources function in th=
e ZBC standard,=0A=
>>> + * with the addition of max active zones support (added in the ZNS sta=
ndard).=0A=
>>> + *=0A=
>>> + * The function determines if a zone can transition to implicit open o=
r explicit open,=0A=
>>> + * while maintaining the max open zone (and max active zone) limit(s).=
 It may close an=0A=
>>> + * implicit open zone in order to make additional zone resources avail=
able.=0A=
>>> + *=0A=
>>> + * ZBC states that an implicit open zone shall be closed only if there=
 is not=0A=
>>> + * room within the open limit. However, with the addition of an active=
 limit,=0A=
>>> + * it is not certain that closing an implicit open zone will allow a n=
ew zone=0A=
>>> + * to be opened, since we might already be at the active limit capacit=
y.=0A=
>>> + */=0A=
>>> +static bool null_manage_zone_resources(struct nullb_device *dev, struc=
t blk_zone *zone)=0A=
>>=0A=
>> I still do not like the name. Since this return a bool, what about=0A=
>> null_has_zone_resources() ?=0A=
> =0A=
> I also don't like the name :)=0A=
> =0A=
> However, since the ZBC spec, in the descriptions of "Write operation, Fin=
ish=0A=
> operation, and Open operation", says that the "manage open zone resources=
"=0A=
> function must be called before each of these operations are performed,=0A=
> and that there is a section that defines how the "manage open zone resour=
ces"=0A=
> is defined, I was thinking that having a similar name would be of value.=
=0A=
> =0A=
> And I agree that it is weird that it returns a bool, but that is how it i=
s=0A=
> defined in the standard.=0A=
> =0A=
> Perhaps it should have exactly the same name as the standard, i.e.=0A=
> null_manage_open_zone_resources() ?=0A=
> =0A=
> However, if you don't think that there is any point of trying to have=0A=
> a similar name to the function in ZBC, then I will happily rename it :)=
=0A=
=0A=
Well, I prefer to prioritize code readability over following a not-so-good =
name=0A=
that the standard chose. The function description makes it clear that it is=
 zone=0A=
management a-la-ZBC, so a function name clarifying what is being checked is=
=0A=
better in my opinion. Not a blocker though. Feel free to chose what to do h=
ere.=0A=
=0A=
Cheers.=0A=
=0A=
=0A=
-- =0A=
Damien Le Moal=0A=
Western Digital Research=0A=