All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3 rfc] Support discovery log change events
@ 2019-02-23  2:31 Sagi Grimberg
  2019-02-23  2:31 ` [PATCH 1/3 rfc] nvme-fabrics: allow discovery subsystems accept a kato Sagi Grimberg
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Sagi Grimberg @ 2019-02-23  2:31 UTC (permalink / raw)


We want to be able to support discovery log change events automatically
without user intervention.

The definition of discovery log change events applies on "persistent" long
lived controllers, so first we need to have discovery controllers to stay
for a long time and accept kato value.

Then when we do happen to get a discovery log change event on the persistent
discovery controller, we simply fire a udev event to user-space to re-query
the discovery log page and connect to new subsystems in the fabric.

Review and feedback is welcome.

Sagi Grimberg (3):
  nvme-fabrics: allow discovery subsystems accept a kato
  nvme: enable aen also for discovery controllers
  nvme: fire discovery log page change events to userspace

 drivers/nvme/host/core.c    | 37 ++++++++++++++++++++++++++++++++++---
 drivers/nvme/host/fabrics.c | 12 ++----------
 2 files changed, 36 insertions(+), 13 deletions(-)

-- 
2.17.1

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

* [PATCH 1/3 rfc] nvme-fabrics: allow discovery subsystems accept a kato
  2019-02-23  2:31 [PATCH 0/3 rfc] Support discovery log change events Sagi Grimberg
@ 2019-02-23  2:31 ` Sagi Grimberg
  2019-02-24 17:13   ` Hannes Reinecke
  2019-02-23  2:31 ` [PATCH 2/3 rfc] nvme: enable aen also for discovery controllers Sagi Grimberg
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Sagi Grimberg @ 2019-02-23  2:31 UTC (permalink / raw)


This modifies the behavior of discovery subsystems to accept
a kato as a preparation to support discovery log change
events. This also means that now every discovery controller
will have a default kato value, and for non-persistent connections
the host needs to pass in a zero kato value (keep_alive_tmo=0).

Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 drivers/nvme/host/fabrics.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index 70c09abcfcbf..9d53ea4939cd 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -389,8 +389,8 @@ int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl)
 	 * Set keep-alive timeout in seconds granularity (ms * 1000)
 	 * and add a grace period for controller kato enforcement
 	 */
-	cmd.connect.kato = ctrl->opts->discovery_nqn ? 0 :
-		cpu_to_le32((ctrl->kato + NVME_KATO_GRACE) * 1000);
+	cmd.connect.kato = ctrl->kato ?
+		cpu_to_le32((ctrl->kato + NVME_KATO_GRACE) * 1000) : 0;
 
 	if (ctrl->opts->disable_sqflow)
 		cmd.connect.cattr |= NVME_CONNECT_DISABLE_SQFLOW;
@@ -746,13 +746,6 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
 				pr_warn("keep_alive_tmo 0 won't execute keep alives!!!\n");
 			}
 			opts->kato = token;
-
-			if (opts->discovery_nqn && opts->kato) {
-				pr_err("Discovery controllers cannot accept KATO != 0\n");
-				ret = -EINVAL;
-				goto out;
-			}
-
 			break;
 		case NVMF_OPT_CTRL_LOSS_TMO:
 			if (match_int(args, &token)) {
@@ -873,7 +866,6 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
 	}
 
 	if (opts->discovery_nqn) {
-		opts->kato = 0;
 		opts->nr_io_queues = 0;
 		opts->nr_write_queues = 0;
 		opts->nr_poll_queues = 0;
-- 
2.17.1

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

* [PATCH 2/3 rfc] nvme: enable aen also for discovery controllers
  2019-02-23  2:31 [PATCH 0/3 rfc] Support discovery log change events Sagi Grimberg
  2019-02-23  2:31 ` [PATCH 1/3 rfc] nvme-fabrics: allow discovery subsystems accept a kato Sagi Grimberg
@ 2019-02-23  2:31 ` Sagi Grimberg
  2019-02-24 17:14   ` Hannes Reinecke
  2019-02-25  0:07   ` Max Gurtovoy
  2019-02-23  2:31 ` [PATCH 3/3 rfc] nvme: fire discovery log page change events to userspace Sagi Grimberg
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 14+ messages in thread
From: Sagi Grimberg @ 2019-02-23  2:31 UTC (permalink / raw)


If the controller supports discovery log page change events,
we want to enable it on the host side as well.

Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 drivers/nvme/host/core.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 9ec88253ebcd..348af75583e5 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1178,6 +1178,8 @@ static void nvme_enable_aen(struct nvme_ctrl *ctrl)
 	if (!supported_aens)
 		return;
 
+	queue_work(nvme_wq, &ctrl->async_event_work);
+
 	status = nvme_set_features(ctrl, NVME_FEAT_ASYNC_EVENT, supported_aens,
 			NULL, 0, &result);
 	if (status)
@@ -3662,10 +3664,13 @@ void nvme_start_ctrl(struct nvme_ctrl *ctrl)
 	if (ctrl->kato)
 		nvme_start_keep_alive(ctrl);
 
+	if (ctrl->queue_count > 1 ||
+	    (ctrl->ops->flags & NVME_F_FABRICS &&
+	     ctrl->opts->discovery_nqn))
+		nvme_enable_aen(ctrl);
+
 	if (ctrl->queue_count > 1) {
 		nvme_queue_scan(ctrl);
-		nvme_enable_aen(ctrl);
-		queue_work(nvme_wq, &ctrl->async_event_work);
 		nvme_start_queues(ctrl);
 	}
 }
-- 
2.17.1

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

* [PATCH 3/3 rfc] nvme: fire discovery log page change events to userspace
  2019-02-23  2:31 [PATCH 0/3 rfc] Support discovery log change events Sagi Grimberg
  2019-02-23  2:31 ` [PATCH 1/3 rfc] nvme-fabrics: allow discovery subsystems accept a kato Sagi Grimberg
  2019-02-23  2:31 ` [PATCH 2/3 rfc] nvme: enable aen also for discovery controllers Sagi Grimberg
@ 2019-02-23  2:31 ` Sagi Grimberg
  2019-02-24 17:16   ` Hannes Reinecke
  2019-02-23  2:31 ` [PATCH rfc nvme-cli 4/5] fabrics: support persistent connections to a discovery controller Sagi Grimberg
  2019-02-23  2:31 ` [PATCH rfc nvme-cli 5/5] fabrics: allow user to retrieve discovery log from existing " Sagi Grimberg
  4 siblings, 1 reply; 14+ messages in thread
From: Sagi Grimberg @ 2019-02-23  2:31 UTC (permalink / raw)


Provide userspace with nvme discovery controller device instance,
controller traddr and trsvcid. We'd expect userspace to handle
this event by issuing a discovery + connect.

Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 drivers/nvme/host/core.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 348af75583e5..d29ff9b910d9 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1168,7 +1168,8 @@ int nvme_set_queue_count(struct nvme_ctrl *ctrl, int *count)
 EXPORT_SYMBOL_GPL(nvme_set_queue_count);
 
 #define NVME_AEN_SUPPORTED \
-	(NVME_AEN_CFG_NS_ATTR | NVME_AEN_CFG_FW_ACT | NVME_AEN_CFG_ANA_CHANGE)
+	(NVME_AEN_CFG_NS_ATTR | NVME_AEN_CFG_FW_ACT | NVME_AEN_CFG_ANA_CHANGE | \
+	 NVME_AEN_CFG_DISC_CHANGE)
 
 static void nvme_enable_aen(struct nvme_ctrl *ctrl)
 {
@@ -3527,6 +3528,28 @@ static void nvme_aen_uevent(struct nvme_ctrl *ctrl)
 	kfree(envp[0]);
 }
 
+static void nvme_disc_aen_uevent(struct nvme_ctrl *ctrl)
+{
+	struct nvmf_ctrl_options *opts = ctrl->opts;
+	char *envp[16];
+	int i, envloc = 0;
+
+	envp[envloc++] = kasprintf(GFP_KERNEL, "NVME_EVENT=discovery");
+	envp[envloc++] = kasprintf(GFP_KERNEL, "NVME_INSTANCE=%d", ctrl->instance);
+	envp[envloc++] = kasprintf(GFP_KERNEL, "NVME_TRTYPE=%s", opts->transport);
+	envp[envloc++] = kasprintf(GFP_KERNEL, "NVME_TRADDR=%s", opts->traddr);
+	envp[envloc++] = kasprintf(GFP_KERNEL, "NVME_TRSVCID=%s", opts->trsvcid);
+	envp[envloc++] = kasprintf(GFP_KERNEL, "NVME_HOST_TRADDR=%s",
+			opts->host_traddr ?: "none");
+	envp[envloc] = NULL;
+
+	for (i = 0; i < envloc; i++)
+		dev_dbg(ctrl->device, "%s\n", envp[i]);
+	kobject_uevent_env(&ctrl->device->kobj, KOBJ_CHANGE, envp);
+	for (i = 0; i < envloc; i++)
+		kfree(envp[i]);
+}
+
 static void nvme_async_event_work(struct work_struct *work)
 {
 	struct nvme_ctrl *ctrl =
@@ -3618,6 +3641,9 @@ static void nvme_handle_aen_notice(struct nvme_ctrl *ctrl, u32 result)
 		queue_work(nvme_wq, &ctrl->ana_work);
 		break;
 #endif
+	case NVME_AER_NOTICE_DISC_CHANGED:
+		nvme_disc_aen_uevent(ctrl);
+		break;
 	default:
 		dev_warn(ctrl->device, "async event result %08x\n", result);
 	}
-- 
2.17.1

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

* [PATCH rfc nvme-cli 4/5] fabrics: support persistent connections to a discovery controller
  2019-02-23  2:31 [PATCH 0/3 rfc] Support discovery log change events Sagi Grimberg
                   ` (2 preceding siblings ...)
  2019-02-23  2:31 ` [PATCH 3/3 rfc] nvme: fire discovery log page change events to userspace Sagi Grimberg
@ 2019-02-23  2:31 ` Sagi Grimberg
  2019-02-23  2:34   ` Sagi Grimberg
  2019-02-23  2:31 ` [PATCH rfc nvme-cli 5/5] fabrics: allow user to retrieve discovery log from existing " Sagi Grimberg
  4 siblings, 1 reply; 14+ messages in thread
From: Sagi Grimberg @ 2019-02-23  2:31 UTC (permalink / raw)


Simply don't destroy the discovery controller after getting the
log pages. Note that persistent connection to a discovery subsystem
require to pass in a non-zero kato value, so if not provided we
simply use a default of 30 seconds kato.

Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 fabrics.c | 10 +++++++++-
 fabrics.h |  2 ++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/fabrics.c b/fabrics.c
index 029105b227ff..44aee67759c8 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -62,6 +62,7 @@ static struct config {
 	char *raw;
 	char *device;
 	int  duplicate_connect;
+	bool persistent;
 } cfg = { NULL };
 
 #define BUF_SIZE		4096
@@ -743,7 +744,8 @@ static int do_discover(char *argstr, bool connect)
 		return errno;
 	ret = nvmf_get_log_page_discovery(dev_name, &log, &numrec);
 	free(dev_name);
-	remove_ctrl(instance);
+	if (!cfg.persistent)
+		remove_ctrl(instance);
 
 	switch (ret) {
 	case DISC_OK:
@@ -819,6 +821,9 @@ static int discover_from_conf_file(const char *desc, char *argstr,
 		if (err)
 			continue;
 
+		if (cfg.persistent && !cfg.keep_alive_tmo)
+			cfg.keep_alive_tmo = NVMF_DEF_DISC_TMO;
+
 		err = build_options(argstr, BUF_SIZE);
 		if (err) {
 			ret = err;
@@ -855,6 +860,7 @@ int discover(const char *desc, int argc, char **argv, bool connect)
 		{"keep-alive-tmo",  'k', "LIST", CFG_INT, &cfg.keep_alive_tmo,  required_argument, "keep alive timeout period in seconds" },
 		{"reconnect-delay", 'c', "LIST", CFG_INT, &cfg.reconnect_delay, required_argument, "reconnect timeout period in seconds" },
 		{"ctrl-loss-tmo",   'l', "LIST", CFG_INT, &cfg.ctrl_loss_tmo,   required_argument, "controller loss timeout period in seconds" },
+		{"persistent",  'p', "LIST", CFG_NONE, &cfg.persistent,  no_argument, "persistent discovery connection" },
 		{NULL},
 	};
 
@@ -869,6 +875,8 @@ int discover(const char *desc, int argc, char **argv, bool connect)
 		return discover_from_conf_file(desc, argstr,
 				command_line_options, connect);
 	} else {
+		if (cfg.persistent && !cfg.keep_alive_tmo)
+			cfg.keep_alive_tmo = NVMF_DEF_DISC_TMO;
 		ret = build_options(argstr, BUF_SIZE);
 		if (ret)
 			return ret;
diff --git a/fabrics.h b/fabrics.h
index 988f3ef2fbc4..7c1664b80d51 100644
--- a/fabrics.h
+++ b/fabrics.h
@@ -1,6 +1,8 @@
 #ifndef _DISCOVER_H
 #define _DISCOVER_H
 
+#define NVMF_DEF_DISC_TMO	30
+
 extern int discover(const char *desc, int argc, char **argv, bool connect);
 extern int connect(const char *desc, int argc, char **argv);
 extern int disconnect(const char *desc, int argc, char **argv);
-- 
2.17.1

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

* [PATCH rfc nvme-cli 5/5] fabrics: allow user to retrieve discovery log from existing discovery controller
  2019-02-23  2:31 [PATCH 0/3 rfc] Support discovery log change events Sagi Grimberg
                   ` (3 preceding siblings ...)
  2019-02-23  2:31 ` [PATCH rfc nvme-cli 4/5] fabrics: support persistent connections to a discovery controller Sagi Grimberg
@ 2019-02-23  2:31 ` Sagi Grimberg
  2019-02-23  2:34   ` Sagi Grimberg
  4 siblings, 1 reply; 14+ messages in thread
From: Sagi Grimberg @ 2019-02-23  2:31 UTC (permalink / raw)


Simply allow discover to receive the discovery device node name.
Also centralize extraction of controller instance from the controller
name to a common helper.

Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 fabrics.c | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/fabrics.c b/fabrics.c
index 44aee67759c8..703020fa5b17 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -172,6 +172,19 @@ static const char *cms_str(__u8 cm)
 
 static int do_discover(char *argstr, bool connect);
 
+static int ctrl_instance(char *device)
+{
+	int ret, instance;
+
+	device = basename(device);
+	ret = sscanf(device, "nvme%d", &instance);
+	if (ret < 0)
+		return ret;
+	if (!ret)
+		return -1;
+	return instance;
+}
+
 static int add_ctrl(const char *argstr)
 {
 	substring_t args[MAX_OPT_ARGS];
@@ -736,7 +749,10 @@ static int do_discover(char *argstr, bool connect)
 	char *dev_name;
 	int instance, numrec = 0, ret;
 
-	instance = add_ctrl(argstr);
+	if (!cfg.device)
+		instance = add_ctrl(argstr);
+	else
+		instance = ctrl_instance(cfg.device);
 	if (instance < 0)
 		return instance;
 
@@ -744,7 +760,7 @@ static int do_discover(char *argstr, bool connect)
 		return errno;
 	ret = nvmf_get_log_page_discovery(dev_name, &log, &numrec);
 	free(dev_name);
-	if (!cfg.persistent)
+	if (!cfg.device && !cfg.persistent)
 		remove_ctrl(instance);
 
 	switch (ret) {
@@ -857,6 +873,7 @@ int discover(const char *desc, int argc, char **argv, bool connect)
 		{"hostnqn",     'q', "LIST", CFG_STRING, &cfg.hostnqn,     required_argument, "user-defined hostnqn (if default not used)" },
 		{"hostid",      'I', "LIST", CFG_STRING, &cfg.hostid,      required_argument, "user-defined hostid (if default not used)"},
 		{"raw",         'r', "LIST", CFG_STRING, &cfg.raw,         required_argument, "raw output file" },
+		{"device",      'd', "LIST", CFG_STRING, &cfg.device, required_argument, "use existing discovery contoller device" },
 		{"keep-alive-tmo",  'k', "LIST", CFG_INT, &cfg.keep_alive_tmo,  required_argument, "keep alive timeout period in seconds" },
 		{"reconnect-delay", 'c', "LIST", CFG_INT, &cfg.reconnect_delay, required_argument, "reconnect timeout period in seconds" },
 		{"ctrl-loss-tmo",   'l', "LIST", CFG_INT, &cfg.ctrl_loss_tmo,   required_argument, "controller loss timeout period in seconds" },
@@ -1001,15 +1018,10 @@ static int disconnect_by_nqn(char *nqn)
 static int disconnect_by_device(char *device)
 {
 	int instance;
-	int ret;
-
-	device = basename(device);
-	ret = sscanf(device, "nvme%d", &instance);
-	if (ret < 0)
-		return ret;
-	if (!ret)
-		return -1;
 
+	instance = ctrl_instance(device);
+	if (instance < 0)
+		return instance;
 	return remove_ctrl(instance);
 }
 
-- 
2.17.1

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

* [PATCH rfc nvme-cli 4/5] fabrics: support persistent connections to a discovery controller
  2019-02-23  2:31 ` [PATCH rfc nvme-cli 4/5] fabrics: support persistent connections to a discovery controller Sagi Grimberg
@ 2019-02-23  2:34   ` Sagi Grimberg
  0 siblings, 0 replies; 14+ messages in thread
From: Sagi Grimberg @ 2019-02-23  2:34 UTC (permalink / raw)


Please ignore this patch - it is a part of the nvme-cli patch set

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

* [PATCH rfc nvme-cli 5/5] fabrics: allow user to retrieve discovery log from existing discovery controller
  2019-02-23  2:31 ` [PATCH rfc nvme-cli 5/5] fabrics: allow user to retrieve discovery log from existing " Sagi Grimberg
@ 2019-02-23  2:34   ` Sagi Grimberg
  0 siblings, 0 replies; 14+ messages in thread
From: Sagi Grimberg @ 2019-02-23  2:34 UTC (permalink / raw)


Please ignore this patch - it is a part of the nvme-cli patch set

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

* [PATCH 1/3 rfc] nvme-fabrics: allow discovery subsystems accept a kato
  2019-02-23  2:31 ` [PATCH 1/3 rfc] nvme-fabrics: allow discovery subsystems accept a kato Sagi Grimberg
@ 2019-02-24 17:13   ` Hannes Reinecke
  0 siblings, 0 replies; 14+ messages in thread
From: Hannes Reinecke @ 2019-02-24 17:13 UTC (permalink / raw)


On 2/23/19 3:31 AM, Sagi Grimberg wrote:
> This modifies the behavior of discovery subsystems to accept
> a kato as a preparation to support discovery log change
> events. This also means that now every discovery controller
> will have a default kato value, and for non-persistent connections
> the host needs to pass in a zero kato value (keep_alive_tmo=0).
> 
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> ---
>   drivers/nvme/host/fabrics.c | 12 ++----------
>   1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
> index 70c09abcfcbf..9d53ea4939cd 100644
> --- a/drivers/nvme/host/fabrics.c
> +++ b/drivers/nvme/host/fabrics.c
> @@ -389,8 +389,8 @@ int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl)
>   	 * Set keep-alive timeout in seconds granularity (ms * 1000)
>   	 * and add a grace period for controller kato enforcement
>   	 */
> -	cmd.connect.kato = ctrl->opts->discovery_nqn ? 0 :
> -		cpu_to_le32((ctrl->kato + NVME_KATO_GRACE) * 1000);
> +	cmd.connect.kato = ctrl->kato ?
> +		cpu_to_le32((ctrl->kato + NVME_KATO_GRACE) * 1000) : 0;
>   
>   	if (ctrl->opts->disable_sqflow)
>   		cmd.connect.cattr |= NVME_CONNECT_DISABLE_SQFLOW;
> @@ -746,13 +746,6 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
>   				pr_warn("keep_alive_tmo 0 won't execute keep alives!!!\n");
>   			}
>   			opts->kato = token;
> -
> -			if (opts->discovery_nqn && opts->kato) {
> -				pr_err("Discovery controllers cannot accept KATO != 0\n");
> -				ret = -EINVAL;
> -				goto out;
> -			}
> -
>   			break;
>   		case NVMF_OPT_CTRL_LOSS_TMO:
>   			if (match_int(args, &token)) {
> @@ -873,7 +866,6 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
>   	}
>   
>   	if (opts->discovery_nqn) {
> -		opts->kato = 0;
>   		opts->nr_io_queues = 0;
>   		opts->nr_write_queues = 0;
>   		opts->nr_poll_queues = 0;
> 

Reviewed-by: Hannes Reinecke <hare at suse.com>

Cheers,

Hannes

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

* [PATCH 2/3 rfc] nvme: enable aen also for discovery controllers
  2019-02-23  2:31 ` [PATCH 2/3 rfc] nvme: enable aen also for discovery controllers Sagi Grimberg
@ 2019-02-24 17:14   ` Hannes Reinecke
  2019-02-25  0:07   ` Max Gurtovoy
  1 sibling, 0 replies; 14+ messages in thread
From: Hannes Reinecke @ 2019-02-24 17:14 UTC (permalink / raw)


On 2/23/19 3:31 AM, Sagi Grimberg wrote:
> If the controller supports discovery log page change events,
> we want to enable it on the host side as well.
> 
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> ---
>   drivers/nvme/host/core.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 9ec88253ebcd..348af75583e5 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1178,6 +1178,8 @@ static void nvme_enable_aen(struct nvme_ctrl *ctrl)
>   	if (!supported_aens)
>   		return;
>   
> +	queue_work(nvme_wq, &ctrl->async_event_work);
> +
>   	status = nvme_set_features(ctrl, NVME_FEAT_ASYNC_EVENT, supported_aens,
>   			NULL, 0, &result);
>   	if (status)
> @@ -3662,10 +3664,13 @@ void nvme_start_ctrl(struct nvme_ctrl *ctrl)
>   	if (ctrl->kato)
>   		nvme_start_keep_alive(ctrl);
>   
> +	if (ctrl->queue_count > 1 ||
> +	    (ctrl->ops->flags & NVME_F_FABRICS &&
> +	     ctrl->opts->discovery_nqn))
> +		nvme_enable_aen(ctrl);
> +
>   	if (ctrl->queue_count > 1) {
>   		nvme_queue_scan(ctrl);
> -		nvme_enable_aen(ctrl);
> -		queue_work(nvme_wq, &ctrl->async_event_work);
>   		nvme_start_queues(ctrl);
>   	}
>   }
> 
Reviewed-by: Hannes Reinecke <hare at suse.com>

Cheers,

Hannes

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

* [PATCH 3/3 rfc] nvme: fire discovery log page change events to userspace
  2019-02-23  2:31 ` [PATCH 3/3 rfc] nvme: fire discovery log page change events to userspace Sagi Grimberg
@ 2019-02-24 17:16   ` Hannes Reinecke
  2019-02-25 21:25     ` Sagi Grimberg
  0 siblings, 1 reply; 14+ messages in thread
From: Hannes Reinecke @ 2019-02-24 17:16 UTC (permalink / raw)


On 2/23/19 3:31 AM, Sagi Grimberg wrote:
> Provide userspace with nvme discovery controller device instance,
> controller traddr and trsvcid. We'd expect userspace to handle
> this event by issuing a discovery + connect.
> 
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> ---
>   drivers/nvme/host/core.c | 28 +++++++++++++++++++++++++++-
>   1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 348af75583e5..d29ff9b910d9 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1168,7 +1168,8 @@ int nvme_set_queue_count(struct nvme_ctrl *ctrl, int *count)
>   EXPORT_SYMBOL_GPL(nvme_set_queue_count);
>   
>   #define NVME_AEN_SUPPORTED \
> -	(NVME_AEN_CFG_NS_ATTR | NVME_AEN_CFG_FW_ACT | NVME_AEN_CFG_ANA_CHANGE)
> +	(NVME_AEN_CFG_NS_ATTR | NVME_AEN_CFG_FW_ACT | NVME_AEN_CFG_ANA_CHANGE | \
> +	 NVME_AEN_CFG_DISC_CHANGE)
>   
>   static void nvme_enable_aen(struct nvme_ctrl *ctrl)
>   {
> @@ -3527,6 +3528,28 @@ static void nvme_aen_uevent(struct nvme_ctrl *ctrl)
>   	kfree(envp[0]);
>   }
>   
> +static void nvme_disc_aen_uevent(struct nvme_ctrl *ctrl)
> +{
> +	struct nvmf_ctrl_options *opts = ctrl->opts;
> +	char *envp[16];
> +	int i, envloc = 0;
> +
> +	envp[envloc++] = kasprintf(GFP_KERNEL, "NVME_EVENT=discovery");
> +	envp[envloc++] = kasprintf(GFP_KERNEL, "NVME_INSTANCE=%d", ctrl->instance);
> +	envp[envloc++] = kasprintf(GFP_KERNEL, "NVME_TRTYPE=%s", opts->transport);
> +	envp[envloc++] = kasprintf(GFP_KERNEL, "NVME_TRADDR=%s", opts->traddr);
> +	envp[envloc++] = kasprintf(GFP_KERNEL, "NVME_TRSVCID=%s", opts->trsvcid);
> +	envp[envloc++] = kasprintf(GFP_KERNEL, "NVME_HOST_TRADDR=%s",
> +			opts->host_traddr ?: "none");
> +	envp[envloc] = NULL;
> +
trsvcid is also optional, so we should be using

opts->trsvcid ?: "none"

here, too.

Cheers,

Hannes

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

* [PATCH 2/3 rfc] nvme: enable aen also for discovery controllers
  2019-02-23  2:31 ` [PATCH 2/3 rfc] nvme: enable aen also for discovery controllers Sagi Grimberg
  2019-02-24 17:14   ` Hannes Reinecke
@ 2019-02-25  0:07   ` Max Gurtovoy
  2019-02-25 21:24     ` Sagi Grimberg
  1 sibling, 1 reply; 14+ messages in thread
From: Max Gurtovoy @ 2019-02-25  0:07 UTC (permalink / raw)



On 2/23/2019 4:31 AM, Sagi Grimberg wrote:
> If the controller supports discovery log page change events,
> we want to enable it on the host side as well.
>
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> ---
>   drivers/nvme/host/core.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 9ec88253ebcd..348af75583e5 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1178,6 +1178,8 @@ static void nvme_enable_aen(struct nvme_ctrl *ctrl)
>   	if (!supported_aens)
>   		return;
>   
> +	queue_work(nvme_wq, &ctrl->async_event_work);
> +
>   	status = nvme_set_features(ctrl, NVME_FEAT_ASYNC_EVENT, supported_aens,
>   			NULL, 0, &result);
>   	if (status)
> @@ -3662,10 +3664,13 @@ void nvme_start_ctrl(struct nvme_ctrl *ctrl)
>   	if (ctrl->kato)
>   		nvme_start_keep_alive(ctrl);
>   
> +	if (ctrl->queue_count > 1 ||
> +	    (ctrl->ops->flags & NVME_F_FABRICS &&
> +	     ctrl->opts->discovery_nqn))
> +		nvme_enable_aen(ctrl);
> +

why queue_count > 1 is a condition to enable AEN ?

Sounds more like an admin related action...

>   	if (ctrl->queue_count > 1) {
>   		nvme_queue_scan(ctrl);
> -		nvme_enable_aen(ctrl);
> -		queue_work(nvme_wq, &ctrl->async_event_work);
>   		nvme_start_queues(ctrl);
>   	}
>   }

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

* [PATCH 2/3 rfc] nvme: enable aen also for discovery controllers
  2019-02-25  0:07   ` Max Gurtovoy
@ 2019-02-25 21:24     ` Sagi Grimberg
  0 siblings, 0 replies; 14+ messages in thread
From: Sagi Grimberg @ 2019-02-25 21:24 UTC (permalink / raw)



>> @@ -3662,10 +3664,13 @@ void nvme_start_ctrl(struct nvme_ctrl *ctrl)
>> ????? if (ctrl->kato)
>> ????????? nvme_start_keep_alive(ctrl);
>> +??? if (ctrl->queue_count > 1 ||
>> +??????? (ctrl->ops->flags & NVME_F_FABRICS &&
>> +???????? ctrl->opts->discovery_nqn))
>> +??????? nvme_enable_aen(ctrl);
>> +
> 
> why queue_count > 1 is a condition to enable AEN ?

I just pulled the below code up, but it seems you are right
regardless of this patch. The AER should be independent of
having I/O queues.

> Sounds more like an admin related action...
> 
>> ????? if (ctrl->queue_count > 1) {
>> ????????? nvme_queue_scan(ctrl);
>> -??????? nvme_enable_aen(ctrl);
>> -??????? queue_work(nvme_wq, &ctrl->async_event_work);
>> ????????? nvme_start_queues(ctrl);
>> ????? }
>> ? }

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

* [PATCH 3/3 rfc] nvme: fire discovery log page change events to userspace
  2019-02-24 17:16   ` Hannes Reinecke
@ 2019-02-25 21:25     ` Sagi Grimberg
  0 siblings, 0 replies; 14+ messages in thread
From: Sagi Grimberg @ 2019-02-25 21:25 UTC (permalink / raw)



>> @@ -3527,6 +3528,28 @@ static void nvme_aen_uevent(struct nvme_ctrl 
>> *ctrl)
>> ????? kfree(envp[0]);
>> ? }
>> +static void nvme_disc_aen_uevent(struct nvme_ctrl *ctrl)
>> +{
>> +??? struct nvmf_ctrl_options *opts = ctrl->opts;
>> +??? char *envp[16];
>> +??? int i, envloc = 0;
>> +
>> +??? envp[envloc++] = kasprintf(GFP_KERNEL, "NVME_EVENT=discovery");
>> +??? envp[envloc++] = kasprintf(GFP_KERNEL, "NVME_INSTANCE=%d", 
>> ctrl->instance);
>> +??? envp[envloc++] = kasprintf(GFP_KERNEL, "NVME_TRTYPE=%s", 
>> opts->transport);
>> +??? envp[envloc++] = kasprintf(GFP_KERNEL, "NVME_TRADDR=%s", 
>> opts->traddr);
>> +??? envp[envloc++] = kasprintf(GFP_KERNEL, "NVME_TRSVCID=%s", 
>> opts->trsvcid);
>> +??? envp[envloc++] = kasprintf(GFP_KERNEL, "NVME_HOST_TRADDR=%s",
>> +??????????? opts->host_traddr ?: "none");
>> +??? envp[envloc] = NULL;
>> +
> trsvcid is also optional, so we should be using
> 
> opts->trsvcid ?: "none"
> 
> here, too.


Yea, you're right

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

end of thread, other threads:[~2019-02-25 21:25 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-23  2:31 [PATCH 0/3 rfc] Support discovery log change events Sagi Grimberg
2019-02-23  2:31 ` [PATCH 1/3 rfc] nvme-fabrics: allow discovery subsystems accept a kato Sagi Grimberg
2019-02-24 17:13   ` Hannes Reinecke
2019-02-23  2:31 ` [PATCH 2/3 rfc] nvme: enable aen also for discovery controllers Sagi Grimberg
2019-02-24 17:14   ` Hannes Reinecke
2019-02-25  0:07   ` Max Gurtovoy
2019-02-25 21:24     ` Sagi Grimberg
2019-02-23  2:31 ` [PATCH 3/3 rfc] nvme: fire discovery log page change events to userspace Sagi Grimberg
2019-02-24 17:16   ` Hannes Reinecke
2019-02-25 21:25     ` Sagi Grimberg
2019-02-23  2:31 ` [PATCH rfc nvme-cli 4/5] fabrics: support persistent connections to a discovery controller Sagi Grimberg
2019-02-23  2:34   ` Sagi Grimberg
2019-02-23  2:31 ` [PATCH rfc nvme-cli 5/5] fabrics: allow user to retrieve discovery log from existing " Sagi Grimberg
2019-02-23  2:34   ` Sagi Grimberg

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.