linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] nvme: fix multipath crash when ANA desactivated
@ 2019-07-05 14:05 Marta Rybczynska
  2019-07-06 10:06 ` Max Gurtovoy
  0 siblings, 1 reply; 7+ messages in thread
From: Marta Rybczynska @ 2019-07-05 14:05 UTC (permalink / raw)
  To: kbusch, axboe, hch, sagi, linux-nvme, linux-kernel
  Cc: Jean-Baptiste Riaux, Samuel Jones

Fix a crash with multipath activated. It happends when ANA log
page is larger than MDTS and because of that ANA is disabled.
The driver then tries to access unallocated buffer when connecting
to a nvme target. The signature is as follows:

[  300.433586] nvme nvme0: ANA log page size (8208) larger than MDTS (8192).
[  300.435387] nvme nvme0: disabling ANA support.
[  300.437835] nvme nvme0: creating 4 I/O queues.
[  300.459132] nvme nvme0: new ctrl: NQN "nqn.0.0.0", addr 10.91.0.1:8009
[  300.464609] BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
[  300.466342] #PF error: [normal kernel read fault]
[  300.467385] PGD 0 P4D 0
[  300.467987] Oops: 0000 [#1] SMP PTI
[  300.468787] CPU: 3 PID: 50 Comm: kworker/u8:1 Not tainted 5.0.20kalray+ #4
[  300.470264] Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
[  300.471532] Workqueue: nvme-wq nvme_scan_work [nvme_core]
[  300.472724] RIP: 0010:nvme_parse_ana_log+0x21/0x140 [nvme_core]
[  300.474038] Code: 45 01 d2 d8 48 98 c3 66 90 0f 1f 44 00 00 41 57 41 56 41 55 41 54 55 53 48 89 fb 48 83 ec 08 48 8b af 20 0a 00 00 48 89 34 24 <66> 83 7d 08 00 0f 84 c6 00 00 00 44 8b 7d 14 49 89 d5 8b 55 10 48
[  300.477374] RSP: 0018:ffffa50e80fd7cb8 EFLAGS: 00010296
[  300.478334] RAX: 0000000000000001 RBX: ffff9130f1872258 RCX: 0000000000000000
[  300.479784] RDX: ffffffffc06c4c30 RSI: ffff9130edad4280 RDI: ffff9130f1872258
[  300.481488] RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000044
[  300.483203] R10: 0000000000000220 R11: 0000000000000040 R12: ffff9130f18722c0
[  300.484928] R13: ffff9130f18722d0 R14: ffff9130edad4280 R15: ffff9130f18722c0
[  300.486626] FS:  0000000000000000(0000) GS:ffff9130f7b80000(0000) knlGS:0000000000000000
[  300.488538] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  300.489907] CR2: 0000000000000008 CR3: 00000002365e6000 CR4: 00000000000006e0
[  300.491612] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  300.493303] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[  300.494991] Call Trace:
[  300.495645]  nvme_mpath_add_disk+0x5c/0xb0 [nvme_core]
[  300.496880]  nvme_validate_ns+0x2ef/0x550 [nvme_core]
[  300.498105]  ? nvme_identify_ctrl.isra.45+0x6a/0xb0 [nvme_core]
[  300.499539]  nvme_scan_work+0x2b4/0x370 [nvme_core]
[  300.500717]  ? __switch_to_asm+0x35/0x70
[  300.501663]  process_one_work+0x171/0x380
[  300.502340]  worker_thread+0x49/0x3f0
[  300.503079]  kthread+0xf8/0x130
[  300.503795]  ? max_active_store+0x80/0x80
[  300.504690]  ? kthread_bind+0x10/0x10
[  300.505502]  ret_from_fork+0x35/0x40
[  300.506280] Modules linked in: nvme_tcp nvme_rdma rdma_cm iw_cm ib_cm ib_core nvme_fabrics nvme_core xt_physdev ip6table_raw ip6table_mangle ip6table_filter ip6_tables xt_comment iptable_nat nf_nat_ipv4 nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 xt_CHECKSUM iptable_mangle iptable_filter veth ebtable_filter ebtable_nat ebtables iptable_raw vxlan ip6_udp_tunnel udp_tunnel sunrpc joydev pcspkr virtio_balloon br_netfilter bridge stp llc ip_tables xfs libcrc32c ata_generic pata_acpi virtio_net virtio_console net_failover virtio_blk failover ata_piix serio_raw libata virtio_pci virtio_ring virtio
[  300.514984] CR2: 0000000000000008
[  300.515569] ---[ end trace faa2eefad7e7f218 ]---
[  300.516354] RIP: 0010:nvme_parse_ana_log+0x21/0x140 [nvme_core]
[  300.517330] Code: 45 01 d2 d8 48 98 c3 66 90 0f 1f 44 00 00 41 57 41 56 41 55 41 54 55 53 48 89 fb 48 83 ec 08 48 8b af 20 0a 00 00 48 89 34 24 <66> 83 7d 08 00 0f 84 c6 00 00 00 44 8b 7d 14 49 89 d5 8b 55 10 48
[  300.520353] RSP: 0018:ffffa50e80fd7cb8 EFLAGS: 00010296
[  300.521229] RAX: 0000000000000001 RBX: ffff9130f1872258 RCX: 0000000000000000
[  300.522399] RDX: ffffffffc06c4c30 RSI: ffff9130edad4280 RDI: ffff9130f1872258
[  300.523560] RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000044
[  300.524734] R10: 0000000000000220 R11: 0000000000000040 R12: ffff9130f18722c0
[  300.525915] R13: ffff9130f18722d0 R14: ffff9130edad4280 R15: ffff9130f18722c0
[  300.527084] FS:  0000000000000000(0000) GS:ffff9130f7b80000(0000) knlGS:0000000000000000
[  300.528396] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  300.529440] CR2: 0000000000000008 CR3: 00000002365e6000 CR4: 00000000000006e0
[  300.530739] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  300.531989] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[  300.533264] Kernel panic - not syncing: Fatal exception
[  300.534338] Kernel Offset: 0x17c00000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
[  300.536227] ---[ end Kernel panic - not syncing: Fatal exception ]---

Signed-off-by: Marta Rybczynska <marta.rybczynska@kalray.eu>
Tested-by: Jean-Baptiste Riaux <jbriaux@kalray.eu>
---
 drivers/nvme/host/multipath.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 499acf0..5ba982b 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -14,7 +14,8 @@
 
 inline bool nvme_ctrl_use_ana(struct nvme_ctrl *ctrl)
 {
-	return multipath && ctrl->subsys && (ctrl->subsys->cmic & (1 << 3));
+	return multipath && ctrl->ana_log_buf && ctrl->subsys &&
+		(ctrl->subsys->cmic & (1 << 3));
 }
 
 /*
@@ -614,7 +615,10 @@ int nvme_mpath_init(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
 {
 	int error;
 
-	if (!nvme_ctrl_use_ana(ctrl))
+	/* check if multipath is enabled and we have the capability */
+	if (!multipath)
+		return 0;
+	if (!ctrl->subsys || ((ctrl->subsys->cmic & (1 << 3)) != 0))
 		return 0;
 
 	ctrl->anacap = id->anacap;
-- 
1.8.3.1


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

* Re: [PATCH v2] nvme: fix multipath crash when ANA desactivated
  2019-07-05 14:05 [PATCH v2] nvme: fix multipath crash when ANA desactivated Marta Rybczynska
@ 2019-07-06 10:06 ` Max Gurtovoy
  2019-07-09 21:29   ` Christoph Hellwig
  0 siblings, 1 reply; 7+ messages in thread
From: Max Gurtovoy @ 2019-07-06 10:06 UTC (permalink / raw)
  To: Marta Rybczynska, kbusch, axboe, hch, sagi, linux-nvme, linux-kernel
  Cc: Samuel Jones, Jean-Baptiste Riaux


On 7/5/2019 5:05 PM, Marta Rybczynska wrote:
> Fix a crash with multipath activated. It happends when ANA log
> page is larger than MDTS and because of that ANA is disabled.
> The driver then tries to access unallocated buffer when connecting
> to a nvme target. The signature is as follows:
>
> [  300.433586] nvme nvme0: ANA log page size (8208) larger than MDTS (8192).
> [  300.435387] nvme nvme0: disabling ANA support.
> [  300.437835] nvme nvme0: creating 4 I/O queues.
> [  300.459132] nvme nvme0: new ctrl: NQN "nqn.0.0.0", addr 10.91.0.1:8009
> [  300.464609] BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
> [  300.466342] #PF error: [normal kernel read fault]
> [  300.467385] PGD 0 P4D 0
> [  300.467987] Oops: 0000 [#1] SMP PTI
> [  300.468787] CPU: 3 PID: 50 Comm: kworker/u8:1 Not tainted 5.0.20kalray+ #4
> [  300.470264] Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
> [  300.471532] Workqueue: nvme-wq nvme_scan_work [nvme_core]
> [  300.472724] RIP: 0010:nvme_parse_ana_log+0x21/0x140 [nvme_core]
> [  300.474038] Code: 45 01 d2 d8 48 98 c3 66 90 0f 1f 44 00 00 41 57 41 56 41 55 41 54 55 53 48 89 fb 48 83 ec 08 48 8b af 20 0a 00 00 48 89 34 24 <66> 83 7d 08 00 0f 84 c6 00 00 00 44 8b 7d 14 49 89 d5 8b 55 10 48
> [  300.477374] RSP: 0018:ffffa50e80fd7cb8 EFLAGS: 00010296
> [  300.478334] RAX: 0000000000000001 RBX: ffff9130f1872258 RCX: 0000000000000000
> [  300.479784] RDX: ffffffffc06c4c30 RSI: ffff9130edad4280 RDI: ffff9130f1872258
> [  300.481488] RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000044
> [  300.483203] R10: 0000000000000220 R11: 0000000000000040 R12: ffff9130f18722c0
> [  300.484928] R13: ffff9130f18722d0 R14: ffff9130edad4280 R15: ffff9130f18722c0
> [  300.486626] FS:  0000000000000000(0000) GS:ffff9130f7b80000(0000) knlGS:0000000000000000
> [  300.488538] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  300.489907] CR2: 0000000000000008 CR3: 00000002365e6000 CR4: 00000000000006e0
> [  300.491612] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [  300.493303] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [  300.494991] Call Trace:
> [  300.495645]  nvme_mpath_add_disk+0x5c/0xb0 [nvme_core]
> [  300.496880]  nvme_validate_ns+0x2ef/0x550 [nvme_core]
> [  300.498105]  ? nvme_identify_ctrl.isra.45+0x6a/0xb0 [nvme_core]
> [  300.499539]  nvme_scan_work+0x2b4/0x370 [nvme_core]
> [  300.500717]  ? __switch_to_asm+0x35/0x70
> [  300.501663]  process_one_work+0x171/0x380
> [  300.502340]  worker_thread+0x49/0x3f0
> [  300.503079]  kthread+0xf8/0x130
> [  300.503795]  ? max_active_store+0x80/0x80
> [  300.504690]  ? kthread_bind+0x10/0x10
> [  300.505502]  ret_from_fork+0x35/0x40
> [  300.506280] Modules linked in: nvme_tcp nvme_rdma rdma_cm iw_cm ib_cm ib_core nvme_fabrics nvme_core xt_physdev ip6table_raw ip6table_mangle ip6table_filter ip6_tables xt_comment iptable_nat nf_nat_ipv4 nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 xt_CHECKSUM iptable_mangle iptable_filter veth ebtable_filter ebtable_nat ebtables iptable_raw vxlan ip6_udp_tunnel udp_tunnel sunrpc joydev pcspkr virtio_balloon br_netfilter bridge stp llc ip_tables xfs libcrc32c ata_generic pata_acpi virtio_net virtio_console net_failover virtio_blk failover ata_piix serio_raw libata virtio_pci virtio_ring virtio
> [  300.514984] CR2: 0000000000000008
> [  300.515569] ---[ end trace faa2eefad7e7f218 ]---
> [  300.516354] RIP: 0010:nvme_parse_ana_log+0x21/0x140 [nvme_core]
> [  300.517330] Code: 45 01 d2 d8 48 98 c3 66 90 0f 1f 44 00 00 41 57 41 56 41 55 41 54 55 53 48 89 fb 48 83 ec 08 48 8b af 20 0a 00 00 48 89 34 24 <66> 83 7d 08 00 0f 84 c6 00 00 00 44 8b 7d 14 49 89 d5 8b 55 10 48
> [  300.520353] RSP: 0018:ffffa50e80fd7cb8 EFLAGS: 00010296
> [  300.521229] RAX: 0000000000000001 RBX: ffff9130f1872258 RCX: 0000000000000000
> [  300.522399] RDX: ffffffffc06c4c30 RSI: ffff9130edad4280 RDI: ffff9130f1872258
> [  300.523560] RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000044
> [  300.524734] R10: 0000000000000220 R11: 0000000000000040 R12: ffff9130f18722c0
> [  300.525915] R13: ffff9130f18722d0 R14: ffff9130edad4280 R15: ffff9130f18722c0
> [  300.527084] FS:  0000000000000000(0000) GS:ffff9130f7b80000(0000) knlGS:0000000000000000
> [  300.528396] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  300.529440] CR2: 0000000000000008 CR3: 00000002365e6000 CR4: 00000000000006e0
> [  300.530739] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [  300.531989] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [  300.533264] Kernel panic - not syncing: Fatal exception
> [  300.534338] Kernel Offset: 0x17c00000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
> [  300.536227] ---[ end Kernel panic - not syncing: Fatal exception ]---
>
> Signed-off-by: Marta Rybczynska <marta.rybczynska@kalray.eu>
> Tested-by: Jean-Baptiste Riaux <jbriaux@kalray.eu>
> ---
>   drivers/nvme/host/multipath.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index 499acf0..5ba982b 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -14,7 +14,8 @@
>   
>   inline bool nvme_ctrl_use_ana(struct nvme_ctrl *ctrl)
>   {
> -	return multipath && ctrl->subsys && (ctrl->subsys->cmic & (1 << 3));
> +	return multipath && ctrl->ana_log_buf && ctrl->subsys &&
> +		(ctrl->subsys->cmic & (1 << 3));
>   }
>   
>   /*
> @@ -614,7 +615,10 @@ int nvme_mpath_init(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
>   {
>   	int error;
>   
> -	if (!nvme_ctrl_use_ana(ctrl))
> +	/* check if multipath is enabled and we have the capability */
> +	if (!multipath)
> +		return 0;
> +	if (!ctrl->subsys || ((ctrl->subsys->cmic & (1 << 3)) != 0))

shouldn't it be:

if (!ctrl->subsys || ((ctrl->subsys->cmic & (1 << 3)) == 0))

or

if (!ctrl->subsys || !(ctrl->subsys->cmic & (1 << 3)))


Otherwise, you don't really do any initialization and return 0 in case you have the capability, right ?

>   		return 0;
>   
>   	ctrl->anacap = id->anacap;

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

* Re: [PATCH v2] nvme: fix multipath crash when ANA desactivated
  2019-07-06 10:06 ` Max Gurtovoy
@ 2019-07-09 21:29   ` Christoph Hellwig
  2019-07-10  5:26     ` Marta Rybczynska
  2019-07-10  8:53     ` Max Gurtovoy
  0 siblings, 2 replies; 7+ messages in thread
From: Christoph Hellwig @ 2019-07-09 21:29 UTC (permalink / raw)
  To: Max Gurtovoy
  Cc: Marta Rybczynska, kbusch, axboe, hch, sagi, linux-nvme,
	linux-kernel, Samuel Jones, Jean-Baptiste Riaux

On Sat, Jul 06, 2019 at 01:06:44PM +0300, Max Gurtovoy wrote:
>> +	/* check if multipath is enabled and we have the capability */
>> +	if (!multipath)
>> +		return 0;
>> +	if (!ctrl->subsys || ((ctrl->subsys->cmic & (1 << 3)) != 0))
>
> shouldn't it be:
>
> if (!ctrl->subsys || ((ctrl->subsys->cmic & (1 << 3)) == 0))
>
> or
>
> if (!ctrl->subsys || !(ctrl->subsys->cmic & (1 << 3)))
>
>
> Otherwise, you don't really do any initialization and return 0 in case you have the capability, right ?

Yes.  FYI, my idea how to fix this would be something like:

diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index a9a927677970..cdb3e5baa329 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -12,11 +12,6 @@ module_param(multipath, bool, 0444);
 MODULE_PARM_DESC(multipath,
 	"turn on native support for multiple controllers per subsystem");
 
-inline bool nvme_ctrl_use_ana(struct nvme_ctrl *ctrl)
-{
-	return multipath && ctrl->subsys && (ctrl->subsys->cmic & (1 << 3));
-}
-
 /*
  * If multipathing is enabled we need to always use the subsystem instance
  * number for numbering our devices to avoid conflicts between subsystems that
@@ -622,7 +617,7 @@ int nvme_mpath_init(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
 {
 	int error;
 
-	if (!nvme_ctrl_use_ana(ctrl))
+	if (!multipath || !ctrl->subsys || !(ctrl->subsys->cmic & (1 << 3)))
 		return 0;
 
 	ctrl->anacap = id->anacap;
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 716a876119c8..14eca76bec5c 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -485,7 +485,10 @@ extern const struct attribute_group *nvme_ns_id_attr_groups[];
 extern const struct block_device_operations nvme_ns_head_ops;
 
 #ifdef CONFIG_NVME_MULTIPATH
-bool nvme_ctrl_use_ana(struct nvme_ctrl *ctrl);
+static inline bool nvme_ctrl_use_ana(struct nvme_ctrl *ctrl)
+{
+	return ctrl->ana_log_buf != NULL;
+}
 void nvme_set_disk_name(char *disk_name, struct nvme_ns *ns,
 			struct nvme_ctrl *ctrl, int *flags);
 void nvme_failover_req(struct request *req);

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

* Re: [PATCH v2] nvme: fix multipath crash when ANA desactivated
  2019-07-09 21:29   ` Christoph Hellwig
@ 2019-07-10  5:26     ` Marta Rybczynska
  2019-07-10 16:38       ` Christoph Hellwig
  2019-07-10  8:53     ` Max Gurtovoy
  1 sibling, 1 reply; 7+ messages in thread
From: Marta Rybczynska @ 2019-07-10  5:26 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Max Gurtovoy, kbusch, axboe, Sagi Grimberg, linux-nvme,
	linux-kernel, Samuel Jones, Jean-Baptiste Riaux



----- On 9 Jul, 2019, at 23:29, Christoph Hellwig hch@lst.de wrote:

> On Sat, Jul 06, 2019 at 01:06:44PM +0300, Max Gurtovoy wrote:
>>> +	/* check if multipath is enabled and we have the capability */
>>> +	if (!multipath)
>>> +		return 0;
>>> +	if (!ctrl->subsys || ((ctrl->subsys->cmic & (1 << 3)) != 0))
>>
>> shouldn't it be:
>>
>> if (!ctrl->subsys || ((ctrl->subsys->cmic & (1 << 3)) == 0))
>>
>> or
>>
>> if (!ctrl->subsys || !(ctrl->subsys->cmic & (1 << 3)))
>>
>>
>> Otherwise, you don't really do any initialization and return 0 in case you have
>> the capability, right ?
> 
> Yes.  FYI, my idea how to fix this would be something like:

Thanks both, error when changing the condition on my side. I submit the next
version very soon.

Christoph, why would you like to put the use_ana function in the header?
It isn't used anywhere else outside of that file.

Regards,
Marta

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

* Re: [PATCH v2] nvme: fix multipath crash when ANA desactivated
  2019-07-09 21:29   ` Christoph Hellwig
  2019-07-10  5:26     ` Marta Rybczynska
@ 2019-07-10  8:53     ` Max Gurtovoy
  1 sibling, 0 replies; 7+ messages in thread
From: Max Gurtovoy @ 2019-07-10  8:53 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Marta Rybczynska, kbusch, axboe, sagi, linux-nvme, linux-kernel,
	Samuel Jones, Jean-Baptiste Riaux


On 7/10/2019 12:29 AM, Christoph Hellwig wrote:
> On Sat, Jul 06, 2019 at 01:06:44PM +0300, Max Gurtovoy wrote:
>>> +	/* check if multipath is enabled and we have the capability */
>>> +	if (!multipath)
>>> +		return 0;
>>> +	if (!ctrl->subsys || ((ctrl->subsys->cmic & (1 << 3)) != 0))
>> shouldn't it be:
>>
>> if (!ctrl->subsys || ((ctrl->subsys->cmic & (1 << 3)) == 0))
>>
>> or
>>
>> if (!ctrl->subsys || !(ctrl->subsys->cmic & (1 << 3)))
>>
>>
>> Otherwise, you don't really do any initialization and return 0 in case you have the capability, right ?
> Yes.  FYI, my idea how to fix this would be something like:
>
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index a9a927677970..cdb3e5baa329 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -12,11 +12,6 @@ module_param(multipath, bool, 0444);
>   MODULE_PARM_DESC(multipath,
>   	"turn on native support for multiple controllers per subsystem");
>   
> -inline bool nvme_ctrl_use_ana(struct nvme_ctrl *ctrl)
> -{
> -	return multipath && ctrl->subsys && (ctrl->subsys->cmic & (1 << 3));
> -}
> -
>   /*
>    * If multipathing is enabled we need to always use the subsystem instance
>    * number for numbering our devices to avoid conflicts between subsystems that
> @@ -622,7 +617,7 @@ int nvme_mpath_init(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
>   {
>   	int error;
>   
> -	if (!nvme_ctrl_use_ana(ctrl))
> +	if (!multipath || !ctrl->subsys || !(ctrl->subsys->cmic & (1 << 3)))
>   		return 0;
>   
>   	ctrl->anacap = id->anacap;
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 716a876119c8..14eca76bec5c 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -485,7 +485,10 @@ extern const struct attribute_group *nvme_ns_id_attr_groups[];
>   extern const struct block_device_operations nvme_ns_head_ops;
>   
>   #ifdef CONFIG_NVME_MULTIPATH
> -bool nvme_ctrl_use_ana(struct nvme_ctrl *ctrl);
> +static inline bool nvme_ctrl_use_ana(struct nvme_ctrl *ctrl)
> +{
> +	return ctrl->ana_log_buf != NULL;
> +}
>   void nvme_set_disk_name(char *disk_name, struct nvme_ns *ns,
>   			struct nvme_ctrl *ctrl, int *flags);
>   void nvme_failover_req(struct request *req);

Yes this looks good.


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

* Re: [PATCH v2] nvme: fix multipath crash when ANA desactivated
  2019-07-10  5:26     ` Marta Rybczynska
@ 2019-07-10 16:38       ` Christoph Hellwig
  2019-07-12 13:08         ` Marta Rybczynska
  0 siblings, 1 reply; 7+ messages in thread
From: Christoph Hellwig @ 2019-07-10 16:38 UTC (permalink / raw)
  To: Marta Rybczynska
  Cc: Christoph Hellwig, Max Gurtovoy, kbusch, axboe, Sagi Grimberg,
	linux-nvme, linux-kernel, Samuel Jones, Jean-Baptiste Riaux

On Wed, Jul 10, 2019 at 07:26:46AM +0200, Marta Rybczynska wrote:
> Christoph, why would you like to put the use_ana function in the header?
> It isn't used anywhere else outside of that file.

nvme_ctrl_use_ana has a single caller in core.c as well.

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

* Re: [PATCH v2] nvme: fix multipath crash when ANA desactivated
  2019-07-10 16:38       ` Christoph Hellwig
@ 2019-07-12 13:08         ` Marta Rybczynska
  0 siblings, 0 replies; 7+ messages in thread
From: Marta Rybczynska @ 2019-07-12 13:08 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Max Gurtovoy, kbusch, axboe, Sagi Grimberg, linux-nvme,
	linux-kernel, Samuel Jones, Jean-Baptiste Riaux



----- On 10 Jul, 2019, at 18:38, Christoph Hellwig hch@lst.de wrote:

> On Wed, Jul 10, 2019 at 07:26:46AM +0200, Marta Rybczynska wrote:
>> Christoph, why would you like to put the use_ana function in the header?
>> It isn't used anywhere else outside of that file.
> 
> nvme_ctrl_use_ana has a single caller in core.c as well.

Thanks Christoph, I'm testing a version similar to yours.

Marta

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

end of thread, other threads:[~2019-07-12 13:08 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-05 14:05 [PATCH v2] nvme: fix multipath crash when ANA desactivated Marta Rybczynska
2019-07-06 10:06 ` Max Gurtovoy
2019-07-09 21:29   ` Christoph Hellwig
2019-07-10  5:26     ` Marta Rybczynska
2019-07-10 16:38       ` Christoph Hellwig
2019-07-12 13:08         ` Marta Rybczynska
2019-07-10  8:53     ` Max Gurtovoy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).