WARNING: CPU: 1 PID: 495 at mm/slab_common.c:69 kmem_cache_create+0x1a9/0x330()

DISCUSSION poma 9 participants 27 messages Jul 18, 2014 – Jul 31, 2014 1 message not in the archive SCSI

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

poma

author

2014-07-18 11:04 UTC #1 lore raw

I guess someone working over the summertime. :)

------------[ cut here ]------------
WARNING: CPU: 1 PID: 495 at mm/slab_common.c:69 kmem_cache_create+0x1a9/0x330()
Modules linked in: virtio_net virtio_scsi(+) drm virtio_pci i2c_core virtio_ring ata_generic pata_acpi virtio sunrpc dm_crypt dm_round_robin linear raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor xor async_tx raid6_pq raid1 raid0 iscsi_ibft iscsi_boot_sysfs floppy iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi squashfs cramfs edd dm_multipath
CPU: 1 PID: 495 Comm: systemd-udevd Not tainted 3.16.0-0.rc5.git0.1.fc21.x86_64 #1
Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
  0000000000000000 00000000bfd3b05a ffff88007f55fa40 ffffffff8171b4e3
  0000000000000000 ffff88007f55fa78 ffffffff8108f30d ffff880079175d40
  ffff88007f55ffd8 ffffffff81c6de08 ffff88007f55ffd8 ffff88007f55ffd8
Call Trace:
  [<ffffffff8171b4e3>] dump_stack+0x45/0x56
  [<ffffffff8108f30d>] warn_slowpath_common+0x7d/0xa0
  [<ffffffff8108f43a>] warn_slowpath_null+0x1a/0x20
  [<ffffffff811a7c59>] kmem_cache_create+0x1a9/0x330
  [<ffffffff814a4070>] scsi_setup_command_freelist+0x120/0x270
  [<ffffffff814a4d90>] scsi_add_host_with_dma+0x60/0x2b0
  [<ffffffffa017b9c9>] virtscsi_probe+0x269/0x2af [virtio_scsi]
  [<ffffffffa01877c0>] ? vp_reset+0x90/0x90 [virtio_pci]
  [<ffffffffa010e1d9>] virtio_dev_probe+0xe9/0x160 [virtio]
  [<ffffffff81483d83>] driver_probe_device+0xa3/0x400
  [<ffffffff814841ab>] __driver_attach+0x8b/0x90
  [<ffffffff81484120>] ? __device_attach+0x40/0x40
  [<ffffffff81481b23>] bus_for_each_dev+0x73/0xc0
  [<ffffffff8148381e>] driver_attach+0x1e/0x20
  [<ffffffff814833f0>] bus_add_driver+0x180/0x250
  [<ffffffffa0180000>] ? 0xffffffffa017ffff
  [<ffffffff81484974>] driver_register+0x64/0xf0
  [<ffffffffa010e550>] register_virtio_driver+0x20/0x40 [virtio]
  [<ffffffffa0180085>] init+0x85/0x1000 [virtio_scsi]
  [<ffffffff81002148>] do_one_initcall+0xd8/0x210
  [<ffffffff811c3952>] ? __vunmap+0xa2/0x100
  [<ffffffff8110d951>] load_module+0x2061/0x2600
  [<ffffffff811092f0>] ? store_uevent+0x70/0x70
  [<ffffffff8110dfbd>] SyS_init_module+0xcd/0x120
  [<ffffffff817223a9>] system_call_fastpath+0x16/0x1b
---[ end trace 64d7cf025fd3bf4a ]---

https://bugzilla.redhat.com/show_bug.cgi?id=1121092

poma

Slab warns, because the name of the cache being created contains spaces. The "bad" cache is created by scsi_get_host_cmd_pool. Its name (pool->cmd_name) is initialized by scsi_alloc_host_cmd_pool as follows:

	pool->cmd_name = kasprintf(GFP_KERNEL, "%s_cmd", hostt->name);

So, if hostt->name contains spaces, the cache name will also contain spaces and we'll get the warning. And hostt->name can contain spaces, e.g. virtscsi_host_template_single.name="Virtio SCSI HBA".

The issue was introduced by commit 89d9a567952ba ("[SCSI] add support for per-host cmd pools"). It can be fixed e.g. by substituting spaces with underscores in cache names.

Adding the patch author and reviewers to Cc.

Quoted 45 lines · poma

I guess someone working over the summertime. :)

------------[ cut here ]------------
WARNING: CPU: 1 PID: 495 at mm/slab_common.c:69 kmem_cache_create+0x1a9/0x330()
Modules linked in: virtio_net virtio_scsi(+) drm virtio_pci i2c_core virtio_ring ata_generic pata_acpi virtio sunrpc dm_crypt dm_round_robin linear raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor xor async_tx raid6_pq raid1 raid0 iscsi_ibft iscsi_boot_sysfs floppy iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi squashfs cramfs edd dm_multipath
CPU: 1 PID: 495 Comm: systemd-udevd Not tainted 3.16.0-0.rc5.git0.1.fc21.x86_64 #1
Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
  0000000000000000 00000000bfd3b05a ffff88007f55fa40 ffffffff8171b4e3
  0000000000000000 ffff88007f55fa78 ffffffff8108f30d ffff880079175d40
  ffff88007f55ffd8 ffffffff81c6de08 ffff88007f55ffd8 ffff88007f55ffd8
Call Trace:
  [<ffffffff8171b4e3>] dump_stack+0x45/0x56
  [<ffffffff8108f30d>] warn_slowpath_common+0x7d/0xa0
  [<ffffffff8108f43a>] warn_slowpath_null+0x1a/0x20
  [<ffffffff811a7c59>] kmem_cache_create+0x1a9/0x330
  [<ffffffff814a4070>] scsi_setup_command_freelist+0x120/0x270
  [<ffffffff814a4d90>] scsi_add_host_with_dma+0x60/0x2b0
  [<ffffffffa017b9c9>] virtscsi_probe+0x269/0x2af [virtio_scsi]
  [<ffffffffa01877c0>] ? vp_reset+0x90/0x90 [virtio_pci]
  [<ffffffffa010e1d9>] virtio_dev_probe+0xe9/0x160 [virtio]
  [<ffffffff81483d83>] driver_probe_device+0xa3/0x400
  [<ffffffff814841ab>] __driver_attach+0x8b/0x90
  [<ffffffff81484120>] ? __device_attach+0x40/0x40
  [<ffffffff81481b23>] bus_for_each_dev+0x73/0xc0
  [<ffffffff8148381e>] driver_attach+0x1e/0x20
  [<ffffffff814833f0>] bus_add_driver+0x180/0x250
  [<ffffffffa0180000>] ? 0xffffffffa017ffff
  [<ffffffff81484974>] driver_register+0x64/0xf0
  [<ffffffffa010e550>] register_virtio_driver+0x20/0x40 [virtio]
  [<ffffffffa0180085>] init+0x85/0x1000 [virtio_scsi]
  [<ffffffff81002148>] do_one_initcall+0xd8/0x210
  [<ffffffff811c3952>] ? __vunmap+0xa2/0x100
  [<ffffffff8110d951>] load_module+0x2061/0x2600
  [<ffffffff811092f0>] ? store_uevent+0x70/0x70
  [<ffffffff8110dfbd>] SyS_init_module+0xcd/0x120
  [<ffffffff817223a9>] system_call_fastpath+0x16/0x1b
---[ end trace 64d7cf025fd3bf4a ]---

https://bugzilla.redhat.com/show_bug.cgi?id=1121092

poma

Christoph Hellwig

2014-07-18 14:17 UTC #3 lore raw

Quoted 9 lines · Vladimir Davydov

Slab warns, because the name of the cache being created contains spaces. The "bad" cache is created by scsi_get_host_cmd_pool. Its name (pool->cmd_name) is initialized by scsi_alloc_host_cmd_pool as follows:
	pool->cmd_name = kasprintf(GFP_KERNEL, "%s_cmd", hostt->name);
So, if hostt->name contains spaces, the cache name will also contain spaces and we'll get the warning. And hostt->name can contain spaces, e.g. virtscsi_host_template_single.name="Virtio SCSI HBA".

Or might not even be present. I'll send a patch to replace it with ->proc_name, which must not contain spaces and is generally shorter as well.

_______________________________________________
kernel mailing list
[email protected]
https://admin.fedoraproject.org/mailman/listinfo/kernel

poma

author

2014-07-18 20:01 UTC #4 lore raw

Quoted 7 lines · Christoph Hellwig

Quoted 6 lines · Vladimir Davydov
Slab warns, because the name of the cache being created contains spaces. The "bad" cache is created by scsi_get_host_cmd_pool. Its name (pool->cmd_name) is initialized by scsi_alloc_host_cmd_pool as follows:
	pool->cmd_name = kasprintf(GFP_KERNEL, "%s_cmd", hostt->name);

Quoted 8 lines

Quoted 3 lines
So, if hostt->name contains spaces, the cache name will also contain spaces and we'll get the warning. And hostt->name can contain spaces, e.g. virtscsi_host_template_single.name="Virtio SCSI HBA".
Or might not even be present. I'll send a patch to replace it with ->proc_name, which must not contain spaces and is generally shorter as well.

Is this what you thought?
@@ -148,20 +148,20 @@
struct kmem_cache *cmd_slab;
struct kmem_cache *sense_slab;
unsigned int users;
- char *cmd_name;
+ char *proc_name;
char *sense_name;
unsigned int slab_flags;
gfp_t gfp_mask;
};

static struct scsi_host_cmd_pool scsi_cmd_pool = {
- .cmd_name = "scsi_cmd_cache",
+ .proc_name = "scsi_cmd_cache",
.sense_name = "scsi_sense_cache",
.slab_flags = SLAB_HWCACHE_ALIGN,
};

static struct scsi_host_cmd_pool scsi_cmd_dma_pool = {
- .cmd_name = "scsi_cmd_cache(DMA)",
+ .proc_name = "scsi_cmd_cache(DMA)",
.sense_name = "scsi_sense_cache(DMA)",
.slab_flags = SLAB_HWCACHE_ALIGN|SLAB_CACHE_DMA,
.gfp_mask = __GFP_DMA,
@@ -354,7 +354,7 @@
scsi_free_host_cmd_pool(struct scsi_host_cmd_pool *pool) {
kfree(pool->sense_name);
- kfree(pool->cmd_name);
+ kfree(pool->proc_name);
kfree(pool);
}

@@ -368,9 +368,9 @@
if (!pool)
return NULL;

- pool->cmd_name = kasprintf(GFP_KERNEL, "%s_cmd", hostt->name);
+ pool->proc_name = kasprintf(GFP_KERNEL, "%s_cmd", hostt->name);
pool->sense_name = kasprintf(GFP_KERNEL, "%s_sense", hostt->name);
- if (!pool->cmd_name || !pool->sense_name) {
+ if (!pool->proc_name || !pool->sense_name) {
scsi_free_host_cmd_pool(pool);
return NULL;
}
@@ -403,7 +403,7 @@
}

if (!pool->users) {
- pool->cmd_slab = kmem_cache_create(pool->cmd_name, cmd_size, 0,
+ pool->cmd_slab = kmem_cache_create(pool->proc_name, cmd_size, 0,
pool->slab_flags, NULL);
if (!pool->cmd_slab)
goto out_free_pool;

however ain't workin.

poma

_______________________________________________
kernel mailing list
[email protected]
https://admin.fedoraproject.org/mailman/listinfo/kernel

James Bottomley

maintainer

2014-07-18 20:07 UTC #5 lore raw

Quoted 12 lines · poma

Quoted 11 lines · Christoph Hellwig
Quoted 9 lines · Vladimir Davydov
Slab warns, because the name of the cache being created contains spaces. The "bad" cache is created by scsi_get_host_cmd_pool. Its name (pool->cmd_name) is initialized by scsi_alloc_host_cmd_pool as follows:
	pool->cmd_name = kasprintf(GFP_KERNEL, "%s_cmd", hostt->name);
So, if hostt->name contains spaces, the cache name will also contain spaces and we'll get the warning. And hostt->name can contain spaces, e.g. virtscsi_host_template_single.name="Virtio SCSI HBA".

Quoted 6 lines

Quoted 4 lines
Or might not even be present. I'll send a patch to replace it with ->proc_name, which must not contain spaces and is generally shorter as well.
Is this what you thought?

No, he means this, if you want to try it.

James

---

drivers/scsi/scsi.c +2 -2

index 88d46fe..eb07a9b 100644
@@ -368,8 +368,8 @@ scsi_alloc_host_cmd_pool(struct Scsi_Host *shost)
 	if (!pool)
 		return NULL;
 
-	pool->cmd_name = kasprintf(GFP_KERNEL, "%s_cmd", hostt->name);
-	pool->sense_name = kasprintf(GFP_KERNEL, "%s_sense", hostt->name);
+	pool->cmd_name = kasprintf(GFP_KERNEL, "%s_cmd", hostt->proc_name);
+	pool->sense_name = kasprintf(GFP_KERNEL, "%s_sense", hostt->proc_name);
 	if (!pool->cmd_name || !pool->sense_name) {
 		scsi_free_host_cmd_pool(pool);
 		return NULL;

poma

author

2014-07-18 20:16 UTC #6 lore raw

Quoted 33 lines · James Bottomley

Quoted 12 lines · poma
Quoted 11 lines · Christoph Hellwig
Quoted 9 lines · Vladimir Davydov
Slab warns, because the name of the cache being created contains spaces. The "bad" cache is created by scsi_get_host_cmd_pool. Its name (pool->cmd_name) is initialized by scsi_alloc_host_cmd_pool as follows:
	pool->cmd_name = kasprintf(GFP_KERNEL, "%s_cmd", hostt->name);
So, if hostt->name contains spaces, the cache name will also contain spaces and we'll get the warning. And hostt->name can contain spaces, e.g. virtscsi_host_template_single.name="Virtio SCSI HBA".
Quoted 6 lines
Quoted 4 lines
Or might not even be present. I'll send a patch to replace it with ->proc_name, which must not contain spaces and is generally shorter as well.
Is this what you thought?
No, he means this, if you want to try it.
James
---
drivers/scsi/scsi.c +0 -0
index 88d46fe..eb07a9b 100644

drivers/scsi/scsi.c scsi_alloc_host_cmd_pool(struct Scsi_Host *shost) · line 375

Quoted 10 lines

drivers/scsi/scsi.c +2 -2

@@ -368,8 +368,8 @@ scsi_alloc_host_cmd_pool(struct Scsi_Host *shost)
  	if (!pool)
  		return NULL;
 
-	pool->cmd_name = kasprintf(GFP_KERNEL, "%s_cmd", hostt->name);
-	pool->sense_name = kasprintf(GFP_KERNEL, "%s_sense", hostt->name);
+	pool->cmd_name = kasprintf(GFP_KERNEL, "%s_cmd", hostt->proc_name);
+	pool->sense_name = kasprintf(GFP_KERNEL, "%s_sense", hostt->proc_name);
  	if (!pool->cmd_name || !pool->sense_name) {
  		scsi_free_host_cmd_pool(pool);
  		return NULL;

Man, I just now read it correctly - "So, if hostt->name contains spaces". Thanks.

I'll be back.

poma

author

2014-07-18 21:32 UTC #7 lore raw

Quoted 48 lines · poma

Quoted 33 lines · James Bottomley
Quoted 12 lines · poma
Quoted 11 lines · Christoph Hellwig
Quoted 9 lines · Vladimir Davydov
Slab warns, because the name of the cache being created contains spaces. The "bad" cache is created by scsi_get_host_cmd_pool. Its name (pool->cmd_name) is initialized by scsi_alloc_host_cmd_pool as follows:
	pool->cmd_name = kasprintf(GFP_KERNEL, "%s_cmd", hostt->name);
So, if hostt->name contains spaces, the cache name will also contain spaces and we'll get the warning. And hostt->name can contain spaces, e.g. virtscsi_host_template_single.name="Virtio SCSI HBA".
Quoted 6 lines
Quoted 4 lines
Or might not even be present. I'll send a patch to replace it with ->proc_name, which must not contain spaces and is generally shorter as well.
Is this what you thought?
No, he means this, if you want to try it.
James
---
drivers/scsi/scsi.c +0 -0
index 88d46fe..eb07a9b 100644
drivers/scsi/scsi.c scsi_alloc_host_cmd_pool(struct Scsi_Host *shost) · line 375
Quoted 10 lines
drivers/scsi/scsi.c +2 -2
@@ -368,8 +368,8 @@ scsi_alloc_host_cmd_pool(struct Scsi_Host *shost)
   	if (!pool)
   		return NULL;
 
-	pool->cmd_name = kasprintf(GFP_KERNEL, "%s_cmd", hostt->name);
-	pool->sense_name = kasprintf(GFP_KERNEL, "%s_sense", hostt->name);
+	pool->cmd_name = kasprintf(GFP_KERNEL, "%s_cmd", hostt->proc_name);
+	pool->sense_name = kasprintf(GFP_KERNEL, "%s_sense", hostt->proc_name);
   	if (!pool->cmd_name || !pool->sense_name) {
   		scsi_free_host_cmd_pool(pool);
   		return NULL;
Man, I just now read it correctly - "So, if hostt->name contains spaces". Thanks.

Quoted 2 lines

I'll be back.

Yea, I can confirm it works! :)
No warnings.

poma

_______________________________________________
kernel mailing list
[email protected]
https://admin.fedoraproject.org/mailman/listinfo/kernel

Christoph Hellwig

2014-07-19 16:44 UTC #8 lore raw

In reply to James Bottomley

Quoted 3 lines · James Bottomley

Quoted 1 line
Is this what you thought?
No, he means this, if you want to try it.

Yes, that's what I mean.

poma

author

2014-07-21 09:39 UTC #9 lore raw

Quoted 7 lines · Christoph Hellwig

Quoted 3 lines · James Bottomley
Quoted 1 line
Is this what you thought?
No, he means this, if you want to try it.
Yes, that's what I mean.

Thanks for the tip.
Is there a patch somewhere?

poma

_______________________________________________
kernel mailing list
[email protected]
https://admin.fedoraproject.org/mailman/listinfo/kernel

Christoph Hellwig

2014-07-21 14:58 UTC #10 lore raw

Quoted 2 lines · poma

Thanks for the tip.
Is there a patch somewhere?

James sent the patch earlier and you replied to it.

_______________________________________________
kernel mailing list
[email protected]
https://admin.fedoraproject.org/mailman/listinfo/kernel

poma

author

2014-07-21 15:38 UTC #11 lore raw

Quoted 6 lines · Christoph Hellwig

Quoted 2 lines · poma
Thanks for the tip.
Is there a patch somewhere?
James sent the patch earlier and you replied to it.

Yea I could be more precise. :)
What I thought, is the patch pushed in some official repo and officially approved? I guess that's the only way to achieve validity.

poma

_______________________________________________
kernel mailing list
[email protected]
https://admin.fedoraproject.org/mailman/listinfo/kernel

Christoph Hellwig

Tested-by Vladimir Davydov

2014-07-26 16:21 UTC #12 lore raw

In reply to poma

Here's a formal one.

James, can I get your signoff for it?

Vladimir, can I get a reviewed-by from you (or anyone else)?

---
From 73b1034ab1418e2dea75ccf642bc85c728b57313 Mon Sep 17 00:00:00 2001
From: James Bottomley <[email protected]>
Date: Sat, 26 Jul 2014 12:21:26 -0400
Subject: scsi: use short driver name for per-driver cmd slab caches

hostt->name might contain space, so use the ->proc_name short name instead when creating per-driver command slabs.

Reported-by Vladimir Davydov <[email protected]>

Tested-by Vladimir Davydov <[email protected]>

---
 drivers/scsi/scsi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/scsi/scsi.c +2 -2

index 33318f5..df33060 100644
@@ -365,8 +365,8 @@ scsi_alloc_host_cmd_pool(struct Scsi_Host *shost)
 	if (!pool)
 		return NULL;
 
-	pool->cmd_name = kasprintf(GFP_KERNEL, "%s_cmd", hostt->name);
-	pool->sense_name = kasprintf(GFP_KERNEL, "%s_sense", hostt->name);
+	pool->cmd_name = kasprintf(GFP_KERNEL, "%s_cmd", hostt->proc_name);
+	pool->sense_name = kasprintf(GFP_KERNEL, "%s_sense", hostt->proc_name);
 	if (!pool->cmd_name || !pool->sense_name) {
 		scsi_free_host_cmd_pool(pool);
 		return NULL;

Signature · 6 lines

1.9.1

_______________________________________________
kernel mailing list
[email protected]
https://admin.fedoraproject.org/mailman/listinfo/kernel

Martin K. Petersen

Reviewed-by

2014-07-26 16:44 UTC #13 lore raw

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Christoph" == Christoph Hellwig <[email protected]> writes:

Christoph> Here's a formal one. James, can I get your signoff for it?

Christoph> Vladimir, can I get a reviewed-by from you (or anyone else)?

Reviewed-by Martin K. Petersen <[email protected]>

Signature · 1 line

Martin K. Petersen	Oracle Linux Engineering

Vladimir Davydov

Reviewed-by

2014-07-27 08:10 UTC #14 lore raw

In reply to Christoph Hellwig

Quoted 12 lines · Christoph Hellwig

Here's a formal one.
James, can I get your signoff for it?
Vladimir, can I get a reviewed-by from you (or anyone else)?
---
From 73b1034ab1418e2dea75ccf642bc85c728b57313 Mon Sep 17 00:00:00 2001
From: James Bottomley <[email protected]>
Date: Sat, 26 Jul 2014 12:21:26 -0400
Subject: scsi: use short driver name for per-driver cmd slab caches

Quoted 5 lines

hostt->name might contain space, so use the ->proc_name short name instead when creating per-driver command slabs.
Reported-by Vladimir Davydov <[email protected]>
Tested-by Vladimir Davydov <[email protected]>

Actually, it was "poma <[email protected]>", not me, who reported the issue and tested the patch. Please fix the tags.

Reviewed-by Vladimir Davydov <[email protected]>

Quoted 22 lines

---
 drivers/scsi/scsi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/scsi/scsi.c +2 -2

index 33318f5..df33060 100644
@@ -365,8 +365,8 @@ scsi_alloc_host_cmd_pool(struct Scsi_Host *shost)
 	if (!pool)
 		return NULL;
 
-	pool->cmd_name = kasprintf(GFP_KERNEL, "%s_cmd", hostt->name);
-	pool->sense_name = kasprintf(GFP_KERNEL, "%s_sense", hostt->name);
+	pool->cmd_name = kasprintf(GFP_KERNEL, "%s_cmd", hostt->proc_name);
+	pool->sense_name = kasprintf(GFP_KERNEL, "%s_sense", hostt->proc_name);
 	if (!pool->cmd_name || !pool->sense_name) {
 		scsi_free_host_cmd_pool(pool);
 		return NULL;

Signature · 1 line

1.9.1

James Bottomley

maintainer

2014-07-28 07:49 UTC #15 lore raw

In reply to Christoph Hellwig

Quoted 3 lines · Christoph Hellwig

Here's a formal one.
James, can I get your signoff for it?

Sure:

Signed-off-by James Bottomley <[email protected]>

Quoted 5 lines

Vladimir, can I get a reviewed-by from you (or anyone else)?
---
Quoted 1 line
From 73b1034ab1418e2dea75ccf642bc85c728b57313 Mon Sep 17 00:00:00 2001
From: James Bottomley <[email protected]>

That needs to be

From: James Bottomley <[email protected]>

As well. I do list handling on hansenpartnership.com to minimise exchange wreckage on mailinglists, but I should acknowledge Parallels as supporting the work I do.

James

Quoted 27 lines

Date: Sat, 26 Jul 2014 12:21:26 -0400
Subject: scsi: use short driver name for per-driver cmd slab caches
hostt->name might contain space, so use the ->proc_name short name instead when creating per-driver command slabs.
Reported-by Vladimir Davydov <[email protected]>
Tested-by Vladimir Davydov <[email protected]>
---
 drivers/scsi/scsi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/scsi/scsi.c +2 -2
index 33318f5..df33060 100644
@@ -365,8 +365,8 @@ scsi_alloc_host_cmd_pool(struct Scsi_Host *shost)
 	if (!pool)
 		return NULL;
 
-	pool->cmd_name = kasprintf(GFP_KERNEL, "%s_cmd", hostt->name);
-	pool->sense_name = kasprintf(GFP_KERNEL, "%s_sense", hostt->name);
+	pool->cmd_name = kasprintf(GFP_KERNEL, "%s_cmd", hostt->proc_name);
+	pool->sense_name = kasprintf(GFP_KERNEL, "%s_sense", hostt->proc_name);
 	if (!pool->cmd_name || !pool->sense_name) {
 		scsi_free_host_cmd_pool(pool);
 		return NULL;

Christoph Hellwig

2014-07-29 12:26 UTC #16 lore raw

Quoted 7 lines · James Bottomley

That needs to be
From: James Bottomley <[email protected]>
As well. I do list handling on hansenpartnership.com to minimise exchange wreckage on mailinglists, but I should acknowledge Parallels as supporting the work I do.

Thanks, I've update the author, added a Cc to ћtable and pushed it out to the core-for-3.17 branch.

poma

author

2014-07-29 23:25 UTC #17 lore raw

Quoted 12 lines · Christoph Hellwig

Quoted 7 lines · James Bottomley
That needs to be
From: James Bottomley <[email protected]>
As well. I do list handling on hansenpartnership.com to minimise exchange wreckage on mailinglists, but I should acknowledge Parallels as supporting the work I do.
Thanks, I've update the author, added a Cc to ћtable and pushed it out to the core-for-3.17 branch.

Thanks guys!

Ref.
http://git.infradead.org/users/hch/scsi-queue.git/patch/884ffee?hp=16acf5d

poma

_______________________________________________
kernel mailing list
[email protected]
https://admin.fedoraproject.org/mailman/listinfo/kernel

Josh Boyer

2014-07-30 12:21 UTC #18 lore raw

In reply to Christoph Hellwig

Quoted 11 lines · Christoph Hellwig

Quoted 7 lines · James Bottomley
That needs to be
From: James Bottomley <[email protected]>
As well. I do list handling on hansenpartnership.com to minimise exchange wreckage on mailinglists, but I should acknowledge Parallels as supporting the work I do.
Thanks, I've update the author, added a Cc to ћtable and pushed it out to the core-for-3.17 branch.

This fixes a bug in the 3.16 kernel. Why wouldn't it be sent to Linus for inclusion in the final release there?

josh
_______________________________________________
kernel mailing list
[email protected]
https://admin.fedoraproject.org/mailman/listinfo/kernel

Christoph Hellwig

2014-07-30 14:52 UTC #19 lore raw

Quoted 5 lines · Josh Boyer

Quoted 2 lines
Thanks, I've update the author, added a Cc to ћtable and pushed it out to the core-for-3.17 branch.
This fixes a bug in the 3.16 kernel. Why wouldn't it be sent to Linus for inclusion in the final release there?

I'm only collecting patches for scsi, but James remains formal maintainer, so I don't send anything to Linus. Given that delays between me committing things, them getting picked up by James and finally making it to Linux-next I don't feel like another for-3.16 branch at this point is easily workable.

If James wants to cherry pick it and send it on at this time that might still work fine, but he seems fairly busy.

_______________________________________________
kernel mailing list
[email protected]
https://admin.fedoraproject.org/mailman/listinfo/kernel

James Bottomley

maintainer

2014-07-30 15:05 UTC #20 lore raw

Quoted 11 lines · Christoph Hellwig

Quoted 5 lines · Josh Boyer
Quoted 2 lines
Thanks, I've update the author, added a Cc to ћtable and pushed it out to the core-for-3.17 branch.
This fixes a bug in the 3.16 kernel. Why wouldn't it be sent to Linus for inclusion in the final release there?
I'm only collecting patches for scsi, but James remains formal maintainer, so I don't send anything to Linus. Given that delays between me committing things, them getting picked up by James and finally making it to Linux-next I don't feel like another for-3.16 branch at this point is easily workable.

It's been remarkably current, I believe ... it's already up to date. However, we only have 2 -next builds between now and the anticipated 3.16 release (unless Linus does another -rc) so there's not enough time for a patch that isn't already in (although this one is).

Quoted 2 lines

If James wants to cherry pick it and send it on at this time that might still work fine, but he seems fairly busy.

I'll pull it out and refactor the tree.

James

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Christoph Hellwig

2014-07-30 16:15 UTC #21 lore raw

Quoted 1 line · James Bottomley

It's been remarkably current, I believe ... it's already up to date.

I've updated the drivers tree today and it will get a few more updates さoon.

Quoted 3 lines

However, we only have 2 -next builds between now and the anticipated 3.16 release (unless Linus does another -rc) so there's not enough time for a patch that isn't already in (although this one is).

Yes, that's why I'm not eager to complicate things.

Quoted 4 lines

Quoted 2 lines
If James wants to cherry pick it and send it on at this time that might still work fine, but he seems fairly busy.
I'll pull it out and refactor the tree.

I'd rather stop rebasing this close to the release. Linus never had problems if he got a commit twice due to a cherry pick.

Mike Christie

2014-07-30 17:22 UTC #22 lore raw

In reply to Christoph Hellwig

Quoted 26 lines · Christoph Hellwig

Here's a formal one.
James, can I get your signoff for it?
Vladimir, can I get a reviewed-by from you (or anyone else)?
---
From 73b1034ab1418e2dea75ccf642bc85c728b57313 Mon Sep 17 00:00:00 2001
From: James Bottomley <[email protected]>
Date: Sat, 26 Jul 2014 12:21:26 -0400
Subject: scsi: use short driver name for per-driver cmd slab caches
hostt->name might contain space, so use the ->proc_name short name instead when creating per-driver command slabs.
Reported-by Vladimir Davydov <[email protected]>
Tested-by Vladimir Davydov <[email protected]>
---
 drivers/scsi/scsi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/scsi/scsi.c +0 -0
index 33318f5..df33060 100644

drivers/scsi/scsi.c scsi_alloc_host_cmd_pool(struct Scsi_Host *shost) · line 372

Quoted 10 lines

drivers/scsi/scsi.c +2 -2

@@ -365,8 +365,8 @@ scsi_alloc_host_cmd_pool(struct Scsi_Host *shost)
 	if (!pool)
 		return NULL;
 
-	pool->cmd_name = kasprintf(GFP_KERNEL, "%s_cmd", hostt->name);
-	pool->sense_name = kasprintf(GFP_KERNEL, "%s_sense", hostt->name);
+	pool->cmd_name = kasprintf(GFP_KERNEL, "%s_cmd", hostt->proc_name);
+	pool->sense_name = kasprintf(GFP_KERNEL, "%s_sense", hostt->proc_name);
 	if (!pool->cmd_name || !pool->sense_name) {
 		scsi_free_host_cmd_pool(pool);
 		return NULL;

Some drivers like qla2xxx do not set proc_name. I think if 2 drivers like that are loaded then you will hit some other warns/bugs in the kmem cache setup code right?

Christoph Hellwig

2014-07-30 18:00 UTC #23 lore raw

Quoted 3 lines · Mike Christie

Some drivers like qla2xxx do not set proc_name. I think if 2 drivers like that are loaded then you will hit some other warns/bugs in the kmem cache setup code right?

Drivers have to opt into using their own caches by setting .cmd_size in the host template. We could enforce they set ->proc_name for them, but I'd rather keep the amount of sanity checks low unless there's a real need.

James Bottomley

maintainer

2014-07-31 13:02 UTC #24 lore raw

Quoted 9 lines · Christoph Hellwig

Quoted 3 lines · Mike Christie
Some drivers like qla2xxx do not set proc_name. I think if 2 drivers like that are loaded then you will hit some other warns/bugs in the kmem cache setup code right?
Drivers have to opt into using their own caches by setting .cmd_size in the host template. We could enforce they set ->proc_name for them, but I'd rather keep the amount of sanity checks low unless there's a real need.

OK, so lets send this through normal merge window then we get time to see if there's a problem before the backport to stable happens.

James

_______________________________________________
kernel mailing list
[email protected]
https://admin.fedoraproject.org/mailman/listinfo/kernel

Christoph Lameter

2014-07-18 14:20 UTC #25 lore raw

In reply to poma

Quoted 1 line · poma

I guess someone working over the summertime. :)

Cache names should not contain blanks. I guess the

WARN_ON(strchr(name, ' ')); /* It confuses parsers */

was triggered?

poma

author

2014-07-18 20:04 UTC #26 lore raw

Quoted 10 lines · Christoph Lameter

Quoted 1 line · poma
I guess someone working over the summertime. :)
Cache names should not contain blanks. I guess the
WARN_ON(strchr(name, ' ')); /* It confuses parsers */
was triggered?

I can only guess also. ;)

poma

author

2014-07-18 21:35 UTC #27 lore raw

Quoted 5 lines · poma

Quoted 4 lines · Christoph Lameter
Quoted 1 line · poma
I guess someone working over the summertime. :)

Quoted 9 lines

Quoted 6 lines
Cache names should not contain blanks. I guess the
WARN_ON(strchr(name, ' ')); /* It confuses parsers */
was triggered?
I can only guess also. ;)

BTW sorry for the noise. :)

poma