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.
On Fri, Jul 18, 2014 at 12:57:25PM +0200, poma wrote:
>
> 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
>
>
On Fri, Jul 18, 2014 at 05:21:04PM +0400, Vladimir Davydov wrote:
> 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.
On Fri, 18 Jul 2014, poma wrote:
> 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?
On 18.07.2014 16:17, Christoph Hellwig wrote:
> On Fri, Jul 18, 2014 at 05:21:04PM +0400, Vladimir Davydov wrote:
>> 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.
>
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
On 18.07.2014 16:20, Christoph Lameter wrote:
> On Fri, 18 Jul 2014, poma wrote:
>
>> 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
On Fri, 2014-07-18 at 22:01 +0200, poma wrote:
> On 18.07.2014 16:17, Christoph Hellwig wrote:
> > On Fri, Jul 18, 2014 at 05:21:04PM +0400, Vladimir Davydov wrote:
> >> 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.
> >
>
> Is this what you thought?
No, he means this, if you want to try it.
James
---
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 88d46fe..eb07a9b 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -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;
On 18.07.2014 22:07, James Bottomley wrote:
> On Fri, 2014-07-18 at 22:01 +0200, poma wrote:
>> On 18.07.2014 16:17, Christoph Hellwig wrote:
>>> On Fri, Jul 18, 2014 at 05:21:04PM +0400, Vladimir Davydov wrote:
>>>> 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.
>>>
>>
>> Is this what you thought?
>
> No, he means this, if you want to try it.
>
> James
>
> ---
>
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index 88d46fe..eb07a9b 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -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.
On 18.07.2014 22:16, poma wrote:
> On 18.07.2014 22:07, James Bottomley wrote:
>> On Fri, 2014-07-18 at 22:01 +0200, poma wrote:
>>> On 18.07.2014 16:17, Christoph Hellwig wrote:
>>>> On Fri, Jul 18, 2014 at 05:21:04PM +0400, Vladimir Davydov wrote:
>>>>> 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.
>>>>
>>>
>>> Is this what you thought?
>>
>> No, he means this, if you want to try it.
>>
>> James
>>
>> ---
>>
>> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
>> index 88d46fe..eb07a9b 100644
>> --- a/drivers/scsi/scsi.c
>> +++ b/drivers/scsi/scsi.c
>> @@ -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.
>
Yea, I can confirm it works! :)
No warnings.
poma
On 18.07.2014 22:03, poma wrote:
> On 18.07.2014 16:20, Christoph Lameter wrote:
>> On Fri, 18 Jul 2014, poma wrote:
>>
>>> 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. ;)
>
BTW sorry for the noise. :)
poma
On Fri, Jul 18, 2014 at 01:07:26PM -0700, James Bottomley wrote:
> > Is this what you thought?
>
> No, he means this, if you want to try it.
Yes, that's what I mean.
On 19.07.2014 18:44, Christoph Hellwig wrote:
> On Fri, Jul 18, 2014 at 01:07:26PM -0700, James Bottomley wrote:
>>> 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
On Mon, Jul 21, 2014 at 11:39:15AM +0200, poma wrote:
> Thanks for the tip.
> Is there a patch somewhere?
James sent the patch earlier and you replied to it.
On 21.07.2014 16:58, Christoph Hellwig wrote:
> On Mon, Jul 21, 2014 at 11:39:15AM +0200, poma wrote:
>> 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
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(-)
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 33318f5..df33060 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -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;
--
1.9.1
>>>>> "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]>
--
Martin K. Petersen Oracle Linux Engineering
On Sat, Jul 26, 2014 at 06:21:02PM +0200, Christoph Hellwig wrote:
> 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]>
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]>
> ---
> drivers/scsi/scsi.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index 33318f5..df33060 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -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;
> --
> 1.9.1
>
On Sat, 2014-07-26 at 18:21 +0200, Christoph Hellwig wrote:
> Here's a formal one.
>
> James, can I get your signoff for it?
Sure:
Signed-off-by: James Bottomley <[email protected]>
> 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]>
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
> 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(-)
>
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index 33318f5..df33060 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -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;
On Mon, Jul 28, 2014 at 11:49:22AM +0400, James Bottomley wrote:
> 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.
On 29.07.2014 14:26, Christoph Hellwig wrote:
> On Mon, Jul 28, 2014 at 11:49:22AM +0400, James Bottomley wrote:
>> 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
On Tue, Jul 29, 2014 at 02:26:11PM +0200, Christoph Hellwig wrote:
> On Mon, Jul 28, 2014 at 11:49:22AM +0400, James Bottomley wrote:
> > 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
On Wed, Jul 30, 2014 at 08:21:35AM -0400, Josh Boyer wrote:
> > 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.
On Wed, 2014-07-30 at 16:52 +0200, Christoph Hellwig wrote:
> On Wed, Jul 30, 2014 at 08:21:35AM -0400, Josh Boyer wrote:
> > > 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).
> 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
On Wed, Jul 30, 2014 at 07:04:59PM +0400, James Bottomley wrote:
> 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.
> 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.
> > 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.
On 07/26/2014 11:21 AM, Christoph Hellwig wrote:
> 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(-)
>
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index 33318f5..df33060 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -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?
On Wed, Jul 30, 2014 at 12:22:11PM -0500, Mike Christie wrote:
> 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.
On Wed, 2014-07-30 at 19:59 +0200, Christoph Hellwig wrote:
> On Wed, Jul 30, 2014 at 12:22:11PM -0500, Mike Christie wrote:
> > 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