Linux-NVME Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] scan_work improvement
@ 2026-04-25 13:04 Tokunori Ikegami
  2026-04-25 13:04 ` [PATCH v3 1/2] nvme: delete unnecessary empty lines Tokunori Ikegami
  2026-04-25 13:04 ` [PATCH v3 2/2] nvme: initialize known effects to set ns_mgmt NCC and ns_attach NIC Tokunori Ikegami
  0 siblings, 2 replies; 7+ messages in thread
From: Tokunori Ikegami @ 2026-04-25 13:04 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-nvme, Tokunori Ikegami

By the scan_work ns_list is scanned then updated to latest NS state.
For the ns_attach and ns_mgmt commands ns_list update needed always.
Then initialize known effects to set ns_mgmt NCC and ns_attach NIC.

Tokunori Ikegami (2):
  nvme: delete unnecessary empty lines
  nvme: initialize known effects to set ns_mgmt NCC and ns_attach NIC

 drivers/nvme/host/apple.c |  3 +--
 drivers/nvme/host/core.c  | 20 +++++++++++++++-----
 drivers/nvme/host/ioctl.c |  1 -
 3 files changed, 16 insertions(+), 8 deletions(-)

-- 
2.51.0



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3 1/2] nvme: delete unnecessary empty lines
  2026-04-25 13:04 [PATCH v3 0/2] scan_work improvement Tokunori Ikegami
@ 2026-04-25 13:04 ` Tokunori Ikegami
  2026-04-25 13:04 ` [PATCH v3 2/2] nvme: initialize known effects to set ns_mgmt NCC and ns_attach NIC Tokunori Ikegami
  1 sibling, 0 replies; 7+ messages in thread
From: Tokunori Ikegami @ 2026-04-25 13:04 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-nvme, Tokunori Ikegami

Also fix incorrect indentation errors.

Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com>
---
Changes since v1:
- Fix the incorrect indentation for the continuation of the condition.

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

diff --git a/drivers/nvme/host/apple.c b/drivers/nvme/host/apple.c
index ed61b97fde59..8e583ef05d53 100644
--- a/drivers/nvme/host/apple.c
+++ b/drivers/nvme/host/apple.c
@@ -1060,8 +1060,7 @@ static void apple_nvme_reset_work(struct work_struct *work)
 	 * or the previous stage shut it down cleanly.
 	 */
 	if (!(readl(anv->mmio_coproc + APPLE_ANS_COPROC_CPU_CONTROL) &
-		APPLE_ANS_COPROC_CPU_CONTROL_RUN)) {
-
+	    APPLE_ANS_COPROC_CPU_CONTROL_RUN)) {
 		ret = reset_control_assert(anv->reset);
 		if (ret)
 			goto out;
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 1e33af94c24b..1f973d88c830 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1774,7 +1774,6 @@ static void nvme_enable_aen(struct nvme_ctrl *ctrl)
 
 static int nvme_ns_open(struct nvme_ns *ns)
 {
-
 	/* should never be called due to GENHD_FL_HIDDEN */
 	if (WARN_ON_ONCE(nvme_ns_head_multipath(ns->head)))
 		goto fail;
@@ -1793,7 +1792,6 @@ static int nvme_ns_open(struct nvme_ns *ns)
 
 static void nvme_ns_release(struct nvme_ns *ns)
 {
-
 	module_put(ns->ctrl->ops->module);
 	nvme_put_ns(ns);
 }
@@ -4684,14 +4682,13 @@ static void nvme_async_event_work(struct work_struct *work)
 	 * The transport drivers must guarantee AER submission here is safe by
 	 * flushing ctrl async_event_work after changing the controller state
 	 * from LIVE and before freeing the admin queue.
-	*/
+	 */
 	if (nvme_ctrl_state(ctrl) == NVME_CTRL_LIVE)
 		ctrl->ops->submit_async_event(ctrl);
 }
 
 static bool nvme_ctrl_pp_status(struct nvme_ctrl *ctrl)
 {
-
 	u32 csts;
 
 	if (ctrl->ops->reg_read32(ctrl, NVME_REG_CSTS, &csts))
diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index 9597a87cf05d..a5545dab70a8 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -638,7 +638,6 @@ long nvme_ns_chr_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 static int nvme_uring_cmd_checks(unsigned int issue_flags)
 {
-
 	/* NVMe passthrough requires big SQE/CQE support */
 	if ((issue_flags & (IO_URING_F_SQE128|IO_URING_F_CQE32)) !=
 	    (IO_URING_F_SQE128|IO_URING_F_CQE32))
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 2/2] nvme: initialize known effects to set ns_mgmt NCC and ns_attach NIC
  2026-04-25 13:04 [PATCH v3 0/2] scan_work improvement Tokunori Ikegami
  2026-04-25 13:04 ` [PATCH v3 1/2] nvme: delete unnecessary empty lines Tokunori Ikegami
@ 2026-04-25 13:04 ` Tokunori Ikegami
  2026-04-28  6:51   ` Keith Busch
  1 sibling, 1 reply; 7+ messages in thread
From: Tokunori Ikegami @ 2026-04-25 13:04 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-nvme, Tokunori Ikegami

This is to make sure scan_work done for the commands.
Since nvme_passthru_end called when effects set.
Also scan_work is flushed when NIC or NCC effects set.

The change is to improvement the scan_work to make sure.
Always do scan for the ns_mgmt and ns_attach commands.
All drives supporting ns_mgmt and ns_attach should have the entries.
But no need to be depended on it as always do scan.

Signed-off-by: Tokunori Ikegami <ikegami.t@gmail.com>
---
Changes since v2:
- Document the change explanation in both the commit message and code comments.

 drivers/nvme/host/core.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 1f973d88c830..d88813dfaf9c 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3448,7 +3448,7 @@ static int nvme_init_effects_log(struct nvme_ctrl *ctrl,
 
 static void nvme_init_known_nvm_effects(struct nvme_ctrl *ctrl)
 {
-	struct nvme_effects_log	*log = ctrl->effects;
+	struct nvme_effects_log *log = ctrl->effects;
 
 	log->acs[nvme_admin_format_nvm] |= cpu_to_le32(NVME_CMD_EFFECTS_LBCC |
 						NVME_CMD_EFFECTS_NCC |
@@ -3473,6 +3473,19 @@ static void nvme_init_known_nvm_effects(struct nvme_ctrl *ctrl)
 	 */
 	log->acs[nvme_admin_security_recv] &= cpu_to_le32(~NVME_CMD_EFFECTS_CSE_MASK);
 
+	/*
+	 * This is to make sure scan_work done for the commands.
+	 * Since nvme_passthru_end called when effects set.
+	 * Also scan_work is flushed when NIC or NCC effects set.
+	 *
+	 * The change is to improvement the scan_work to make sure.
+	 * Always do scan for the ns_mgmt and ns_attach commands.
+	 * All drives supporting ns_mgmt and ns_attach should have the entries.
+	 * But no need to be depended on it as always do scan.
+	 */
+	log->acs[nvme_admin_ns_mgmt] |= cpu_to_le32(NVME_CMD_EFFECTS_NCC);
+	log->acs[nvme_admin_ns_attach] |= cpu_to_le32(NVME_CMD_EFFECTS_NIC);
+
 	log->iocs[nvme_cmd_write] |= cpu_to_le32(NVME_CMD_EFFECTS_LBCC);
 	log->iocs[nvme_cmd_write_zeroes] |= cpu_to_le32(NVME_CMD_EFFECTS_LBCC);
 	log->iocs[nvme_cmd_write_uncor] |= cpu_to_le32(NVME_CMD_EFFECTS_LBCC);
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 2/2] nvme: initialize known effects to set ns_mgmt NCC and ns_attach NIC
  2026-04-25 13:04 ` [PATCH v3 2/2] nvme: initialize known effects to set ns_mgmt NCC and ns_attach NIC Tokunori Ikegami
@ 2026-04-28  6:51   ` Keith Busch
  2026-04-29  6:08     ` Tokunori Ikegami
  0 siblings, 1 reply; 7+ messages in thread
From: Keith Busch @ 2026-04-28  6:51 UTC (permalink / raw)
  To: Tokunori Ikegami; +Cc: Christoph Hellwig, linux-nvme

On Sat, Apr 25, 2026 at 10:04:13PM +0900, Tokunori Ikegami wrote:
> This is to make sure scan_work done for the commands.
> Since nvme_passthru_end called when effects set.
> Also scan_work is flushed when NIC or NCC effects set.
> 
> The change is to improvement the scan_work to make sure.
> Always do scan for the ns_mgmt and ns_attach commands.
> All drives supporting ns_mgmt and ns_attach should have the entries.
> But no need to be depended on it as always do scan.

I suppose that's why the "known_effects" exists, but it'd be
dissappointing if we really need this. The other opcodes in there
existed before the Command Effects Log was created, so understandable
that controllers may not implement it. But NS management/attach were
introduced after the log, and there is also the AEN that the driver
could rescan on receipt, so it's doubly bad if a controller
implementation really needs this.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 2/2] nvme: initialize known effects to set ns_mgmt NCC and ns_attach NIC
  2026-04-28  6:51   ` Keith Busch
@ 2026-04-29  6:08     ` Tokunori Ikegami
  2026-04-29  8:48       ` Keith Busch
  0 siblings, 1 reply; 7+ messages in thread
From: Tokunori Ikegami @ 2026-04-29  6:08 UTC (permalink / raw)
  To: Keith Busch; +Cc: Christoph Hellwig, linux-nvme

On 2026/04/28 15:51, Keith Busch wrote:
> I suppose that's why the "known_effects" exists, but it'd be
> dissappointing if we really need this. The other opcodes in there
> existed before the Command Effects Log was created, so understandable
> that controllers may not implement it. But NS management/attach were
> introduced after the log, and there is also the AEN that the driver
> could rescan on receipt, so it's doubly bad if a controller
> implementation really needs this.

Just rechecked the specification again then the log page identifier: 
commands supported and effects mentioned is optional for NVM express 
revision 1.4 and earlier then for the older controller the changes not 
bad I think.
By the way about the scan_work executions for the command effects log 
and AEN I thought it is better to be handled only once but I do not have 
an actual idea for the exclusive control.
Thank you.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 2/2] nvme: initialize known effects to set ns_mgmt NCC and ns_attach NIC
  2026-04-29  6:08     ` Tokunori Ikegami
@ 2026-04-29  8:48       ` Keith Busch
  2026-04-29 12:49         ` Tokunori Ikegami
  0 siblings, 1 reply; 7+ messages in thread
From: Keith Busch @ 2026-04-29  8:48 UTC (permalink / raw)
  To: Tokunori Ikegami; +Cc: Christoph Hellwig, linux-nvme

On Wed, Apr 29, 2026 at 03:08:59PM +0900, Tokunori Ikegami wrote:
> On 2026/04/28 15:51, Keith Busch wrote:
> > I suppose that's why the "known_effects" exists, but it'd be
> > dissappointing if we really need this. The other opcodes in there
> > existed before the Command Effects Log was created, so understandable
> > that controllers may not implement it. But NS management/attach were
> > introduced after the log, and there is also the AEN that the driver
> > could rescan on receipt, so it's doubly bad if a controller
> > implementation really needs this.
> 
> Just rechecked the specification again then the log page identifier:
> commands supported and effects mentioned is optional for NVM express
> revision 1.4 and earlier then for the older controller the changes not bad I
> think.

Yes, but the namespace management commands are also optional, so it
sounds weird to me for an implementation to choose to support the
optional command that's more difficult to do and skip the easy log page
that advertises support for it.

> By the way about the scan_work executions for the command effects log and
> AEN I thought it is better to be handled only once but I do not have an
> actual idea for the exclusive control.

You can attach namespaces to controllers driven by a different host, so
the AEN is the only way to trigger a rescan in such a scenario without
admin intervention. An extra scan_work is otherwise harmless, I don't
see a particular need to introduce a mechanism to avoid it.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 2/2] nvme: initialize known effects to set ns_mgmt NCC and ns_attach NIC
  2026-04-29  8:48       ` Keith Busch
@ 2026-04-29 12:49         ` Tokunori Ikegami
  0 siblings, 0 replies; 7+ messages in thread
From: Tokunori Ikegami @ 2026-04-29 12:49 UTC (permalink / raw)
  To: Keith Busch; +Cc: Christoph Hellwig, linux-nvme

On 2026/04/29 17:48, Keith Busch wrote:
> Yes, but the namespace management commands are also optional, so it
> sounds weird to me for an implementation to choose to support the
> optional command that's more difficult to do and skip the easy log page
> that advertises support for it.
Okay understood as the changes not necessary.
> You can attach namespaces to controllers driven by a different host, so
> the AEN is the only way to trigger a rescan in such a scenario without
> admin intervention. An extra scan_work is otherwise harmless, I don't
> see a particular need to introduce a mechanism to avoid it.

Noted this also. Thank you.




^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-04-29 12:49 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-25 13:04 [PATCH v3 0/2] scan_work improvement Tokunori Ikegami
2026-04-25 13:04 ` [PATCH v3 1/2] nvme: delete unnecessary empty lines Tokunori Ikegami
2026-04-25 13:04 ` [PATCH v3 2/2] nvme: initialize known effects to set ns_mgmt NCC and ns_attach NIC Tokunori Ikegami
2026-04-28  6:51   ` Keith Busch
2026-04-29  6:08     ` Tokunori Ikegami
2026-04-29  8:48       ` Keith Busch
2026-04-29 12:49         ` Tokunori Ikegami

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox