All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pankaj Raghav <p.raghav@samsung.com>
To: "Javier González" <javier.gonz@samsung.com>,
	"Keith Busch" <kbusch@kernel.org>,
	"Christoph Hellwig" <hch@lst.de>,
	"Sagi Grimberg" <sagi@grimberg.me>,
	"Damien Le Moal" <damien.lemoal@opensource.wdc.com>,
	"Jens Axboe" <axboe@kernel.dk>
Cc: Luis Chamberlain <mcgrof@kernel.org>,
	Adam Manzanares <a.manzanares@samsung.com>,
	kanchan Joshi <joshi.k@samsung.com>,
	Johannes Thumshirn <johannes.thumshirn@wdc.com>,
	Pankaj Raghav <pankydev8@gmail.com>,
	Kanchan Joshi <joshiiitr@gmail.com>,
	linux-nvme@lists.infradead.org,
	Pankaj Raghav <p.raghav@samsung.com>
Subject: [PATCH v2] nvme: Fix zns drives without append support to export correct permissions
Date: Wed, 16 Mar 2022 10:34:23 +0100	[thread overview]
Message-ID: <20220316093423.17906-1-p.raghav@samsung.com> (raw)
In-Reply-To: CGME20220316093439eucas1p27c2a31849e9dec6c6bc217c5f1b26180@eucas1p2.samsung.com

This commit 2f4c9ba23b88 ("nvme: export zoned namespaces without Zone
Append support read-only") exported zoned namespaces without append support
to be marked as ro. It does it by setting NVME_NS_FORCE_RO to the
ns->flags in nvme_update_zone_info and later nvme_update_disk_info will
check for this flag and set the disk as ro.

But later this commit 73d90386b559 ("nvme: cleanup zone information
initialization") rearranged nvme_update_disk_info to be called before
nvme_update_zone_info thereby not marking the disk as ro. The call order
cannot be just reverted because nvme_update_zone_info sets certain queue
parameters such as zone_write_granularity that depend on the prior call
to nvme_update_disk_info.

Add a helper nvme_set_disk_ro that can be called in nvme_update_zone_info
to set the permission for ZNS drives correctly.

Fixes: 73d90386b559 ("nvme: cleanup zone information initialization")
Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
Changes since v1:
- Add a new helper to set permission directly from nvme_update_zone_info
  instead of calling nvme_update_disk_info again (Damien)

Note:
Christoph made a point that there is a race window where the disk will
be marked as writable during revalidate zones.
I already responded to the comment in the email thread as it will not be
the case as we mark the disk as ro before we start revalidating the
disk.
https://lore.kernel.org/all/1bbc81a0-19f0-433b-28c2-b22d28176e37@grimberg.me/T/#m0e19022babda81339188ee334551a5fb867abf4c

 drivers/nvme/host/core.c | 3 +--
 drivers/nvme/host/nvme.h | 5 +++++
 drivers/nvme/host/zns.c  | 1 +
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 51c08f206cbf..cde33f2a3a5a 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1855,8 +1855,7 @@ static void nvme_update_disk_info(struct gendisk *disk,
 	blk_queue_max_write_zeroes_sectors(disk->queue,
 					   ns->ctrl->max_zeroes_sectors);
 
-	set_disk_ro(disk, (id->nsattr & NVME_NS_ATTR_RO) ||
-		test_bit(NVME_NS_FORCE_RO, &ns->flags));
+	set_disk_ro(disk, (id->nsattr & NVME_NS_ATTR_RO));
 }
 
 static inline bool nvme_first_scan(struct gendisk *disk)
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index e7ccdb119ede..b6800bdd6ea9 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -607,6 +607,11 @@ static inline bool nvme_is_path_error(u16 status)
 	return (status & 0x700) == 0x300;
 }
 
+static inline void nvme_set_disk_mode_ro(struct nvme_ns *ns)
+{
+	set_disk_ro(ns->disk, test_bit(NVME_NS_FORCE_RO, &ns->flags));
+}
+
 /*
  * Fill in the status and result information from the CQE, and then figure out
  * if blk-mq will need to use IPI magic to complete the request, and if yes do
diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index 9f81beb4df4e..4ab685fa02b4 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -113,6 +113,7 @@ int nvme_update_zone_info(struct nvme_ns *ns, unsigned lbaf)
 	blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
 	blk_queue_max_open_zones(q, le32_to_cpu(id->mor) + 1);
 	blk_queue_max_active_zones(q, le32_to_cpu(id->mar) + 1);
+	nvme_set_disk_mode_ro(ns);
 free_data:
 	kfree(id);
 	return status;
-- 
2.25.1



       reply	other threads:[~2022-03-16  9:34 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20220316093439eucas1p27c2a31849e9dec6c6bc217c5f1b26180@eucas1p2.samsung.com>
2022-03-16  9:34 ` Pankaj Raghav [this message]
2022-03-16  9:54   ` [PATCH v2] nvme: Fix zns drives without append support to export correct permissions Chaitanya Kulkarni
2022-03-16  9:56     ` Chaitanya Kulkarni
2022-03-16 10:21       ` Pankaj Raghav
2022-03-16 16:50         ` Chaitanya Kulkarni
2022-03-16 18:03           ` Pankaj Raghav
2022-03-16 10:50   ` Christoph Hellwig
2022-03-16 14:28     ` Pankaj Raghav
2022-03-22  8:39       ` Christoph Hellwig
2022-03-16 16:52   ` Chaitanya Kulkarni
2022-03-16 18:00     ` Pankaj Raghav

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220316093423.17906-1-p.raghav@samsung.com \
    --to=p.raghav@samsung.com \
    --cc=a.manzanares@samsung.com \
    --cc=axboe@kernel.dk \
    --cc=damien.lemoal@opensource.wdc.com \
    --cc=hch@lst.de \
    --cc=javier.gonz@samsung.com \
    --cc=johannes.thumshirn@wdc.com \
    --cc=joshi.k@samsung.com \
    --cc=joshiiitr@gmail.com \
    --cc=kbusch@kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=mcgrof@kernel.org \
    --cc=pankydev8@gmail.com \
    --cc=sagi@grimberg.me \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.