Message ID | 20230628190649.11233-2-nmi@metaspace.dk |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp9150282vqr; Wed, 28 Jun 2023 12:13:35 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4dExLiMSqY5znAbt5xDz4CWWjcRV6gMt7gXAnGdcu2WQ4xAebrhMgKS1c6J++h6IaHSqVt X-Received: by 2002:a05:651c:220a:b0:2b6:9e4d:119e with SMTP id y10-20020a05651c220a00b002b69e4d119emr592245ljq.2.1687979615078; Wed, 28 Jun 2023 12:13:35 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1687979615; cv=none; d=google.com; s=arc-20160816; b=zJXznSmHaaTIqT2kR4rMnGATMClOAhHNkp4sOTNykaDber99XQlN7UnAiSAqhPsj1z hRnQzPueN4WOFbC8oBYxoIb2KeiafZcWpmIj4TVcsIMQYOOjdAmeOL/zIaHgbMaYWy5h uxAfgK5YKPmn9w9pM3RsmoPcFyMTsHCDvjT97bzCKJnqmPBzSOo4yAQN1Jpe37nwoEic sCeyKHJ7JdfRdvuNoK5Njx+ZuIbZxo4tJhQPXefoJECHAI/VSBkTzwILm6QmGF6jKVHj q7soVeIydVuuyWMlJjnORG0n86TTEMnYAmq0K+8sTLWlFEJSZcj9/ZavpxCiZhV/vMEC cCtg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=1+mgtaI49ANVecSJsMlgWiAuBSlWzk7jhi+QFImWVd0=; fh=PtLHm9B0+hM1tQPeQT3fpvfpYSsUVoUxZslaqHBKa8M=; b=jGJO87IbwU7PdA2ff/Ic3Wepld/pcqq1Di5UULOr5zT0aIujOiQpSgOkJGduHCOZbD smdihIV9lNGA3pzIfuydxanAMWpRmjwX81kxHuPfIHPgR9jDghjHulGn5xxBDCMOQs7T GmG7VbKnB1Yrbk+9iqFkTlcvwYWjBZwmXvyvOKFh6XoriPlYNISiDLsLc1KQ5eSnubwF Z0YAPe9e7wV6ecsdcDHED1L1o6Ylcvx+j7wAn8E44OWJRlFoCP6q0p0XvSc1lRIfuX7z TXJOspc+HyVKRQx6YJit+0TDJSHmphMiHVTMEJH0zNeSX1QpgjGsmPfK7OTAh49PE/aB 9I4A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@metaspace-dk.20221208.gappssmtp.com header.s=20221208 header.b=PkKpRHm8; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id lu13-20020a170906facd00b009929d998ab3si494915ejb.734.2023.06.28.12.13.09; Wed, 28 Jun 2023 12:13:35 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@metaspace-dk.20221208.gappssmtp.com header.s=20221208 header.b=PkKpRHm8; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232124AbjF1THY (ORCPT <rfc822;ivan.orlov0322@gmail.com> + 99 others); Wed, 28 Jun 2023 15:07:24 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55264 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232138AbjF1THA (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 28 Jun 2023 15:07:00 -0400 Received: from mail-lj1-x233.google.com (mail-lj1-x233.google.com [IPv6:2a00:1450:4864:20::233]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 653B6212E for <linux-kernel@vger.kernel.org>; Wed, 28 Jun 2023 12:06:55 -0700 (PDT) Received: by mail-lj1-x233.google.com with SMTP id 38308e7fff4ca-2b5c231c23aso2952511fa.0 for <linux-kernel@vger.kernel.org>; Wed, 28 Jun 2023 12:06:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=metaspace-dk.20221208.gappssmtp.com; s=20221208; t=1687979213; x=1690571213; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=1+mgtaI49ANVecSJsMlgWiAuBSlWzk7jhi+QFImWVd0=; b=PkKpRHm88ty23JmdD6Stv68WY2ScVPPFNR0Y4NXYM3+ohmSQ8JoLfz3P6aKYHyfa3J XtO6/D3dg4RfSDjOC99oQIU0dCtIL8m47OtuB/Qw2NlYnKF3fucYg95jzZESwcJHMZ8w 4FUQ9WeZccmv9k805txFr46dsytAXQBoWqRgDbvNvdp8iDeD59ulFumL4NNN6mIY6K1s q8zmTlCYHivwHdQJQRrdFlCEMi/J5TzNG0SWvsXwcZmgB1aFXmyzy+Uy1xGxCkpeRjTh ZzLqbjKQzATcgk4ldAWg9NOiP90rN4aeFHdOjlLAxqbqBxMX0oBKytS/ofFpMkZmCBn8 rgXw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687979213; x=1690571213; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=1+mgtaI49ANVecSJsMlgWiAuBSlWzk7jhi+QFImWVd0=; b=gFuFtGoLKXlMyaRFFH/VM6Q8WYp80Ji2Wvie5txXMZObaMGHLG0/XPN6nrQQk4uKCu AD/J2+SSOljqtuLiLSwGwLVEgHiMiJEMVkc3oWCVF60Fl3SIondi+y6ej9cNvyKZpUzV ZvmoeH/sBp+pJ7Q557mKlF3R/KJOlzviTCHVuZ7RVwCU0B5ypPGPNjk5/c01NCoBM3CC wH3buNo4XbE5qDHp4/uBHRxPBG6mA51i+ycws0EGEn14+IMBxratjMGmP9nkxNHpS3k8 8QWlG4P/bb/d9+dTh8rLSyFMZE/Jsn+bsWeSFxZnW3UHEfBkBt9aCy+AE3sYU6W6UZoT HsPg== X-Gm-Message-State: AC+VfDxPpaP6zcu45DpiQsWpZzJWQU/t1yfnOuLgVU7olukXs9cqZcFR Oa5Ufzj8bZpyJiYZXWEvbeTMew== X-Received: by 2002:a2e:86c6:0:b0:2b6:c3b8:3a94 with SMTP id n6-20020a2e86c6000000b002b6c3b83a94mr915163ljj.42.1687979213445; Wed, 28 Jun 2023 12:06:53 -0700 (PDT) Received: from localhost ([79.142.230.34]) by smtp.gmail.com with ESMTPSA id ot6-20020a170906ccc600b0098df7d0e096sm5683211ejb.54.2023.06.28.12.06.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 28 Jun 2023 12:06:53 -0700 (PDT) From: Andreas Hindborg <nmi@metaspace.dk> To: Ming Lei <ming.lei@redhat.com> Cc: Hans Holmberg <Hans.Holmberg@wdc.com>, Aravind Ramesh <Aravind.Ramesh@wdc.com>, Jens Axboe <axboe@kernel.dk>, linux-block@vger.kernel.org (open list:BLOCK LAYER), Christoph Hellwig <hch@infradead.org>, Matias Bjorling <Matias.Bjorling@wdc.com>, Andreas Hindborg <a.hindborg@samsung.com>, linux-kernel@vger.kernel.org (open list), Damien Le Moal <dlemoal@kernel.org>, gost.dev@samsung.com, Minwoo Im <minwoo.im.dev@gmail.com> Subject: [PATCH v4 1/4] ublk: change ublk IO command defines to enum Date: Wed, 28 Jun 2023 21:06:46 +0200 Message-ID: <20230628190649.11233-2-nmi@metaspace.dk> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230628190649.11233-1-nmi@metaspace.dk> References: <20230628190649.11233-1-nmi@metaspace.dk> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_NONE, T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1769974912535924848?= X-GMAIL-MSGID: =?utf-8?q?1769974912535924848?= |
Series |
ublk: add zoned storage support
|
|
Commit Message
Andreas Hindborg
June 28, 2023, 7:06 p.m. UTC
From: Andreas Hindborg <a.hindborg@samsung.com> This change is in preparation for zoned storage support. Signed-off-by: Andreas Hindborg <a.hindborg@samsung.com> --- include/uapi/linux/ublk_cmd.h | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-)
Comments
On 6/29/23 04:06, Andreas Hindborg wrote: > From: Andreas Hindborg <a.hindborg@samsung.com> > > This change is in preparation for zoned storage support. > > Signed-off-by: Andreas Hindborg <a.hindborg@samsung.com> > --- > include/uapi/linux/ublk_cmd.h | 23 +++++++++++++++++------ > 1 file changed, 17 insertions(+), 6 deletions(-) > > diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h > index 4b8558db90e1..471b3b983045 100644 > --- a/include/uapi/linux/ublk_cmd.h > +++ b/include/uapi/linux/ublk_cmd.h > @@ -229,12 +229,23 @@ struct ublksrv_ctrl_dev_info { > __u64 reserved2; > }; > > -#define UBLK_IO_OP_READ 0 > -#define UBLK_IO_OP_WRITE 1 > -#define UBLK_IO_OP_FLUSH 2 > -#define UBLK_IO_OP_DISCARD 3 > -#define UBLK_IO_OP_WRITE_SAME 4 > -#define UBLK_IO_OP_WRITE_ZEROES 5 > +enum ublk_op { > + UBLK_IO_OP_READ = 0, > + UBLK_IO_OP_WRITE = 1, > + UBLK_IO_OP_FLUSH = 2, > + UBLK_IO_OP_DISCARD = 3, > + UBLK_IO_OP_WRITE_SAME = 4, > + UBLK_IO_OP_WRITE_ZEROES = 5, > + UBLK_IO_OP_ZONE_OPEN = 10, > + UBLK_IO_OP_ZONE_CLOSE = 11, > + UBLK_IO_OP_ZONE_FINISH = 12, > + UBLK_IO_OP_ZONE_APPEND = 13, > + UBLK_IO_OP_ZONE_RESET = 15, > + __UBLK_IO_OP_DRV_IN_START = 32, > + __UBLK_IO_OP_DRV_IN_END = 96, > + __UBLK_IO_OP_DRV_OUT_START = __UBLK_IO_OP_DRV_IN_END, > + __UBLK_IO_OP_DRV_OUT_END = 160, > +}; This patch does not do what the title says. You are also introducing the zone operations, and the very obscure __UBLK_IO_OP_DRV_XXX operations without an explanation. Also, why the "__" prefix for these ? I do not see the point... Given that this is a uapi, a comment to explain the less obvious commands would be nice. So I think the change to an enum for the existing ops can be done either in patch 2 or as a separate patch and the introduction of the zone operations done in patch 3 or as a separate patch. > > #define UBLK_IO_F_FAILFAST_DEV (1U << 8) > #define UBLK_IO_F_FAILFAST_TRANSPORT (1U << 9)
On Thu, Jun 29, 2023 at 07:47:47AM +0900, Damien Le Moal wrote: > On 6/29/23 04:06, Andreas Hindborg wrote: > > From: Andreas Hindborg <a.hindborg@samsung.com> > > > > This change is in preparation for zoned storage support. > > > > Signed-off-by: Andreas Hindborg <a.hindborg@samsung.com> > > --- > > include/uapi/linux/ublk_cmd.h | 23 +++++++++++++++++------ > > 1 file changed, 17 insertions(+), 6 deletions(-) > > > > diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h > > index 4b8558db90e1..471b3b983045 100644 > > --- a/include/uapi/linux/ublk_cmd.h > > +++ b/include/uapi/linux/ublk_cmd.h > > @@ -229,12 +229,23 @@ struct ublksrv_ctrl_dev_info { > > __u64 reserved2; > > }; > > > > -#define UBLK_IO_OP_READ 0 > > -#define UBLK_IO_OP_WRITE 1 > > -#define UBLK_IO_OP_FLUSH 2 > > -#define UBLK_IO_OP_DISCARD 3 > > -#define UBLK_IO_OP_WRITE_SAME 4 > > -#define UBLK_IO_OP_WRITE_ZEROES 5 > > +enum ublk_op { > > + UBLK_IO_OP_READ = 0, > > + UBLK_IO_OP_WRITE = 1, > > + UBLK_IO_OP_FLUSH = 2, > > + UBLK_IO_OP_DISCARD = 3, > > + UBLK_IO_OP_WRITE_SAME = 4, > > + UBLK_IO_OP_WRITE_ZEROES = 5, > > + UBLK_IO_OP_ZONE_OPEN = 10, > > + UBLK_IO_OP_ZONE_CLOSE = 11, > > + UBLK_IO_OP_ZONE_FINISH = 12, > > + UBLK_IO_OP_ZONE_APPEND = 13, > > + UBLK_IO_OP_ZONE_RESET = 15, > > + __UBLK_IO_OP_DRV_IN_START = 32, > > + __UBLK_IO_OP_DRV_IN_END = 96, > > + __UBLK_IO_OP_DRV_OUT_START = __UBLK_IO_OP_DRV_IN_END, > > + __UBLK_IO_OP_DRV_OUT_END = 160, > > +}; > > This patch does not do what the title says. You are also introducing the zone > operations, and the very obscure __UBLK_IO_OP_DRV_XXX operations without an > explanation. Also, why the "__" prefix for these ? I do not see the point... It should be to reserve space for ublk passthrough OP. > Given that this is a uapi, a comment to explain the less obvious commands would > be nice. > > So I think the change to an enum for the existing ops can be done either in > patch 2 or as a separate patch and the introduction of the zone operations done > in patch 3 or as a separate patch. Also it might break userspace by changing to enum from macro for existed definition, cause userspace may check something by '#ifdef UBLK_IO_OP_*', so probably it is better to keep these OPs as enum, or at least keep existed definition as macro. Thanks, Ming
On 6/29/23 09:38, Ming Lei wrote: > On Thu, Jun 29, 2023 at 07:47:47AM +0900, Damien Le Moal wrote: >> On 6/29/23 04:06, Andreas Hindborg wrote: >>> From: Andreas Hindborg <a.hindborg@samsung.com> >>> >>> This change is in preparation for zoned storage support. >>> >>> Signed-off-by: Andreas Hindborg <a.hindborg@samsung.com> >>> --- >>> include/uapi/linux/ublk_cmd.h | 23 +++++++++++++++++------ >>> 1 file changed, 17 insertions(+), 6 deletions(-) >>> >>> diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h >>> index 4b8558db90e1..471b3b983045 100644 >>> --- a/include/uapi/linux/ublk_cmd.h >>> +++ b/include/uapi/linux/ublk_cmd.h >>> @@ -229,12 +229,23 @@ struct ublksrv_ctrl_dev_info { >>> __u64 reserved2; >>> }; >>> >>> -#define UBLK_IO_OP_READ 0 >>> -#define UBLK_IO_OP_WRITE 1 >>> -#define UBLK_IO_OP_FLUSH 2 >>> -#define UBLK_IO_OP_DISCARD 3 >>> -#define UBLK_IO_OP_WRITE_SAME 4 >>> -#define UBLK_IO_OP_WRITE_ZEROES 5 >>> +enum ublk_op { >>> + UBLK_IO_OP_READ = 0, >>> + UBLK_IO_OP_WRITE = 1, >>> + UBLK_IO_OP_FLUSH = 2, >>> + UBLK_IO_OP_DISCARD = 3, >>> + UBLK_IO_OP_WRITE_SAME = 4, >>> + UBLK_IO_OP_WRITE_ZEROES = 5, >>> + UBLK_IO_OP_ZONE_OPEN = 10, >>> + UBLK_IO_OP_ZONE_CLOSE = 11, >>> + UBLK_IO_OP_ZONE_FINISH = 12, >>> + UBLK_IO_OP_ZONE_APPEND = 13, >>> + UBLK_IO_OP_ZONE_RESET = 15, >>> + __UBLK_IO_OP_DRV_IN_START = 32, >>> + __UBLK_IO_OP_DRV_IN_END = 96, >>> + __UBLK_IO_OP_DRV_OUT_START = __UBLK_IO_OP_DRV_IN_END, >>> + __UBLK_IO_OP_DRV_OUT_END = 160, >>> +}; >> >> This patch does not do what the title says. You are also introducing the zone >> operations, and the very obscure __UBLK_IO_OP_DRV_XXX operations without an >> explanation. Also, why the "__" prefix for these ? I do not see the point... > > It should be to reserve space for ublk passthrough OP. A comment about that would be nice. > >> Given that this is a uapi, a comment to explain the less obvious commands would >> be nice. >> >> So I think the change to an enum for the existing ops can be done either in >> patch 2 or as a separate patch and the introduction of the zone operations done >> in patch 3 or as a separate patch. > > Also it might break userspace by changing to enum from macro for existed > definition, cause userspace may check something by '#ifdef UBLK_IO_OP_*', > so probably it is better to keep these OPs as enum, or at least keep > existed definition as macro. Then let's keep defining things with #define instead of an enum. > > Thanks, > Ming >
Ming Lei <ming.lei@redhat.com> writes: > On Thu, Jun 29, 2023 at 07:47:47AM +0900, Damien Le Moal wrote: >> On 6/29/23 04:06, Andreas Hindborg wrote: >> > From: Andreas Hindborg <a.hindborg@samsung.com> >> > >> > This change is in preparation for zoned storage support. >> > >> > Signed-off-by: Andreas Hindborg <a.hindborg@samsung.com> >> > --- >> > include/uapi/linux/ublk_cmd.h | 23 +++++++++++++++++------ >> > 1 file changed, 17 insertions(+), 6 deletions(-) >> > >> > diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h >> > index 4b8558db90e1..471b3b983045 100644 >> > --- a/include/uapi/linux/ublk_cmd.h >> > +++ b/include/uapi/linux/ublk_cmd.h >> > @@ -229,12 +229,23 @@ struct ublksrv_ctrl_dev_info { >> > __u64 reserved2; >> > }; >> > >> > -#define UBLK_IO_OP_READ 0 >> > -#define UBLK_IO_OP_WRITE 1 >> > -#define UBLK_IO_OP_FLUSH 2 >> > -#define UBLK_IO_OP_DISCARD 3 >> > -#define UBLK_IO_OP_WRITE_SAME 4 >> > -#define UBLK_IO_OP_WRITE_ZEROES 5 >> > +enum ublk_op { >> > + UBLK_IO_OP_READ = 0, >> > + UBLK_IO_OP_WRITE = 1, >> > + UBLK_IO_OP_FLUSH = 2, >> > + UBLK_IO_OP_DISCARD = 3, >> > + UBLK_IO_OP_WRITE_SAME = 4, >> > + UBLK_IO_OP_WRITE_ZEROES = 5, >> > + UBLK_IO_OP_ZONE_OPEN = 10, >> > + UBLK_IO_OP_ZONE_CLOSE = 11, >> > + UBLK_IO_OP_ZONE_FINISH = 12, >> > + UBLK_IO_OP_ZONE_APPEND = 13, >> > + UBLK_IO_OP_ZONE_RESET = 15, >> > + __UBLK_IO_OP_DRV_IN_START = 32, >> > + __UBLK_IO_OP_DRV_IN_END = 96, >> > + __UBLK_IO_OP_DRV_OUT_START = __UBLK_IO_OP_DRV_IN_END, >> > + __UBLK_IO_OP_DRV_OUT_END = 160, >> > +}; >> >> This patch does not do what the title says. You are also introducing the zone >> operations, and the very obscure __UBLK_IO_OP_DRV_XXX operations without an >> explanation. Also, why the "__" prefix for these ? I do not see the point... > > It should be to reserve space for ublk passthrough OP. > >> Given that this is a uapi, a comment to explain the less obvious commands would >> be nice. >> >> So I think the change to an enum for the existing ops can be done either in >> patch 2 or as a separate patch and the introduction of the zone operations done >> in patch 3 or as a separate patch. > > Also it might break userspace by changing to enum from macro for existed > definition, cause userspace may check something by '#ifdef UBLK_IO_OP_*', > so probably it is better to keep these OPs as enum, or at least keep > existed definition as macro. I can change it back to `#define` again, no problem. I only changed it to `enum` on request from Ming [1] Best regards, Andreas [1] https://lore.kernel.org/all/ZAHeWieKXtgYUbvz@ovpn-8-18.pek2.redhat.com/
Damien Le Moal <dlemoal@kernel.org> writes: > On 6/29/23 04:06, Andreas Hindborg wrote: >> From: Andreas Hindborg <a.hindborg@samsung.com> >> >> This change is in preparation for zoned storage support. >> >> Signed-off-by: Andreas Hindborg <a.hindborg@samsung.com> >> --- >> include/uapi/linux/ublk_cmd.h | 23 +++++++++++++++++------ >> 1 file changed, 17 insertions(+), 6 deletions(-) >> >> diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h >> index 4b8558db90e1..471b3b983045 100644 >> --- a/include/uapi/linux/ublk_cmd.h >> +++ b/include/uapi/linux/ublk_cmd.h >> @@ -229,12 +229,23 @@ struct ublksrv_ctrl_dev_info { >> __u64 reserved2; >> }; >> >> -#define UBLK_IO_OP_READ 0 >> -#define UBLK_IO_OP_WRITE 1 >> -#define UBLK_IO_OP_FLUSH 2 >> -#define UBLK_IO_OP_DISCARD 3 >> -#define UBLK_IO_OP_WRITE_SAME 4 >> -#define UBLK_IO_OP_WRITE_ZEROES 5 >> +enum ublk_op { >> + UBLK_IO_OP_READ = 0, >> + UBLK_IO_OP_WRITE = 1, >> + UBLK_IO_OP_FLUSH = 2, >> + UBLK_IO_OP_DISCARD = 3, >> + UBLK_IO_OP_WRITE_SAME = 4, >> + UBLK_IO_OP_WRITE_ZEROES = 5, >> + UBLK_IO_OP_ZONE_OPEN = 10, >> + UBLK_IO_OP_ZONE_CLOSE = 11, >> + UBLK_IO_OP_ZONE_FINISH = 12, >> + UBLK_IO_OP_ZONE_APPEND = 13, >> + UBLK_IO_OP_ZONE_RESET = 15, >> + __UBLK_IO_OP_DRV_IN_START = 32, >> + __UBLK_IO_OP_DRV_IN_END = 96, >> + __UBLK_IO_OP_DRV_OUT_START = __UBLK_IO_OP_DRV_IN_END, >> + __UBLK_IO_OP_DRV_OUT_END = 160, >> +}; > > This patch does not do what the title says. You are also introducing the zone > operations, and the very obscure __UBLK_IO_OP_DRV_XXX operations without an > explanation. Also, why the "__" prefix for these ? I do not see the point... > Given that this is a uapi, a comment to explain the less obvious commands would > be nice. It is a little vague, I'll make sure to include a better description 👍 > > So I think the change to an enum for the existing ops can be done either in > patch 2 or as a separate patch and the introduction of the zone operations done > in patch 3 or as a separate patch. I agree, the zone ops should not be introduced in this patch, I will move them to patch 3. That is a mistake. Best regards, Andreas
> From: Andreas Hindborg <a.hindborg@samsung.com > <mailto:a.hindborg@samsung.com>> > > > This change is in preparation for zoned storage support. > > > Signed-off-by: Andreas Hindborg <a.hindborg@samsung.com > <mailto:a.hindborg@samsung.com>> > --- > include/uapi/linux/ublk_cmd.h | 23 +++++++++++++++++------ > 1 file changed, 17 insertions(+), 6 deletions(-) > > > diff --git a/include/uapi/linux/ublk_cmd.h > b/include/uapi/linux/ublk_cmd.h > index 4b8558db90e1..471b3b983045 100644 > --- a/include/uapi/linux/ublk_cmd.h > +++ b/include/uapi/linux/ublk_cmd.h > @@ -229,12 +229,23 @@ struct ublksrv_ctrl_dev_info { > __u64 reserved2; > }; > > > -#define UBLK_IO_OP_READ 0 > -#define UBLK_IO_OP_WRITE 1 > -#define UBLK_IO_OP_FLUSH 2 > -#define UBLK_IO_OP_DISCARD 3 > -#define UBLK_IO_OP_WRITE_SAME 4 > -#define UBLK_IO_OP_WRITE_ZEROES 5 > +enum ublk_op { > + UBLK_IO_OP_READ = 0, > + UBLK_IO_OP_WRITE = 1, > + UBLK_IO_OP_FLUSH = 2, > + UBLK_IO_OP_DISCARD = 3, > + UBLK_IO_OP_WRITE_SAME = 4, > + UBLK_IO_OP_WRITE_ZEROES = 5, > + UBLK_IO_OP_ZONE_OPEN = 10, > + UBLK_IO_OP_ZONE_CLOSE = 11, > + UBLK_IO_OP_ZONE_FINISH = 12, > + UBLK_IO_OP_ZONE_APPEND = 13, Curious to know if there is a reason to miss enum 14 here ? And if UBLK_IO_OP_ZONE_APPEND is defined along with other operations better to define that in patch 3 itself. > + UBLK_IO_OP_ZONE_RESET = 15, > + __UBLK_IO_OP_DRV_IN_START = 32, > + __UBLK_IO_OP_DRV_IN_END = 96, > + __UBLK_IO_OP_DRV_OUT_START = __UBLK_IO_OP_DRV_IN_END, > + __UBLK_IO_OP_DRV_OUT_END = 160, > +}; > > > #define UBLK_IO_F_FAILFAST_DEV (1U << 8) > #define UBLK_IO_F_FAILFAST_TRANSPORT (1U << 9) Regards, Aravind
aravind.ramesh@opensource.wdc.com writes: >> From: Andreas Hindborg <a.hindborg@samsung.com >> <mailto:a.hindborg@samsung.com>> >> This change is in preparation for zoned storage support. >> Signed-off-by: Andreas Hindborg <a.hindborg@samsung.com >> <mailto:a.hindborg@samsung.com>> >> --- >> include/uapi/linux/ublk_cmd.h | 23 +++++++++++++++++------ >> 1 file changed, 17 insertions(+), 6 deletions(-) >> diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h >> index 4b8558db90e1..471b3b983045 100644 >> --- a/include/uapi/linux/ublk_cmd.h >> +++ b/include/uapi/linux/ublk_cmd.h >> @@ -229,12 +229,23 @@ struct ublksrv_ctrl_dev_info { >> __u64 reserved2; >> }; >> -#define UBLK_IO_OP_READ 0 >> -#define UBLK_IO_OP_WRITE 1 >> -#define UBLK_IO_OP_FLUSH 2 >> -#define UBLK_IO_OP_DISCARD 3 >> -#define UBLK_IO_OP_WRITE_SAME 4 >> -#define UBLK_IO_OP_WRITE_ZEROES 5 >> +enum ublk_op { >> + UBLK_IO_OP_READ = 0, >> + UBLK_IO_OP_WRITE = 1, >> + UBLK_IO_OP_FLUSH = 2, >> + UBLK_IO_OP_DISCARD = 3, >> + UBLK_IO_OP_WRITE_SAME = 4, >> + UBLK_IO_OP_WRITE_ZEROES = 5, >> + UBLK_IO_OP_ZONE_OPEN = 10, >> + UBLK_IO_OP_ZONE_CLOSE = 11, >> + UBLK_IO_OP_ZONE_FINISH = 12, >> + UBLK_IO_OP_ZONE_APPEND = 13, > > Curious to know if there is a reason to miss enum 14 here ? > And if UBLK_IO_OP_ZONE_APPEND is defined along with other operations > better to define that in patch 3 itself. They are defined after REQ_OP_ZONE_*, and they have a hole at 14 🤷 BR Andreas > >> + UBLK_IO_OP_ZONE_RESET = 15, >> + __UBLK_IO_OP_DRV_IN_START = 32, >> + __UBLK_IO_OP_DRV_IN_END = 96, >> + __UBLK_IO_OP_DRV_OUT_START = __UBLK_IO_OP_DRV_IN_END, >> + __UBLK_IO_OP_DRV_OUT_END = 160, >> +}; >> #define UBLK_IO_F_FAILFAST_DEV (1U << 8) >> #define UBLK_IO_F_FAILFAST_TRANSPORT (1U << 9) > > Regards, > Aravind
diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h index 4b8558db90e1..471b3b983045 100644 --- a/include/uapi/linux/ublk_cmd.h +++ b/include/uapi/linux/ublk_cmd.h @@ -229,12 +229,23 @@ struct ublksrv_ctrl_dev_info { __u64 reserved2; }; -#define UBLK_IO_OP_READ 0 -#define UBLK_IO_OP_WRITE 1 -#define UBLK_IO_OP_FLUSH 2 -#define UBLK_IO_OP_DISCARD 3 -#define UBLK_IO_OP_WRITE_SAME 4 -#define UBLK_IO_OP_WRITE_ZEROES 5 +enum ublk_op { + UBLK_IO_OP_READ = 0, + UBLK_IO_OP_WRITE = 1, + UBLK_IO_OP_FLUSH = 2, + UBLK_IO_OP_DISCARD = 3, + UBLK_IO_OP_WRITE_SAME = 4, + UBLK_IO_OP_WRITE_ZEROES = 5, + UBLK_IO_OP_ZONE_OPEN = 10, + UBLK_IO_OP_ZONE_CLOSE = 11, + UBLK_IO_OP_ZONE_FINISH = 12, + UBLK_IO_OP_ZONE_APPEND = 13, + UBLK_IO_OP_ZONE_RESET = 15, + __UBLK_IO_OP_DRV_IN_START = 32, + __UBLK_IO_OP_DRV_IN_END = 96, + __UBLK_IO_OP_DRV_OUT_START = __UBLK_IO_OP_DRV_IN_END, + __UBLK_IO_OP_DRV_OUT_END = 160, +}; #define UBLK_IO_F_FAILFAST_DEV (1U << 8) #define UBLK_IO_F_FAILFAST_TRANSPORT (1U << 9)