Re: [PATCH V5] Save command pool address of Scsi_Host

[Hannes Reinecke]

On 08/04/2014 01:30 PM, jgr...@suse.com wrote:

From: Juergen Gross jgr...@suse.com

If a scsi host driver specifies .cmd_len in it's scsi_host_template, a driver's
private command pool is needed. scsi_find_host_cmd_pool() will locate it, but
scsi_alloc_host_cmd_pool() isn't saving the pool address in the host template.

This will result in an access error when the host is removed.

Avoid the problem by saving the address of a new allocated command pool where
it is expected.

Signed-off-by: Juergen Gross jgr...@suse.com
---
  drivers/scsi/scsi.c | 12 ++--
  1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 88d46fe..b0cef5b 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -380,6 +380,10 @@ scsi_alloc_host_cmd_pool(struct Scsi_Host *shost)
pool-slab_flags |= SLAB_CACHE_DMA;
pool-gfp_mask = __GFP_DMA;
}
+
+   if (hostt-cmd_size)
+   hostt-cmd_pool = pool;
+
return pool;
  }

@@ -424,8 +428,10 @@ out:
  out_free_slab:
kmem_cache_destroy(pool-cmd_slab);
  out_free_pool:
-   if (hostt-cmd_size)
+   if (hostt-cmd_size) {
scsi_free_host_cmd_pool(pool);
+   hostt-cmd_pool = NULL;
+   }
goto out;
  }

@@ -447,8 +453,10 @@ static void scsi_put_host_cmd_pool(struct Scsi_Host *shost)
if (!--pool-users) {
kmem_cache_destroy(pool-cmd_slab);
kmem_cache_destroy(pool-sense_slab);
-   if (hostt-cmd_size)
+   if (hostt-cmd_size) {
scsi_free_host_cmd_pool(pool);
+   hostt-cmd_pool = NULL;
+   }
}
mutex_unlock(host_cmd_pool_mutex);
  }


Reviewed-by: Hannes Reinecke h...@suse.de

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries  Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V5] Save command pool address of Scsi_Host

[Christoph Hellwig]

Can I get another review for this one so I can queue it up for 3.17?
(or v4 if you have a strong preference for it..)

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

[PATCH V5] Save command pool address of Scsi_Host

[jgross]

From: Juergen Gross jgr...@suse.com

If a scsi host driver specifies .cmd_len in it's scsi_host_template, a driver's
private command pool is needed. scsi_find_host_cmd_pool() will locate it, but
scsi_alloc_host_cmd_pool() isn't saving the pool address in the host template.

This will result in an access error when the host is removed.

Avoid the problem by saving the address of a new allocated command pool where
it is expected.

Signed-off-by: Juergen Gross jgr...@suse.com
---
 drivers/scsi/scsi.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 88d46fe..b0cef5b 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -380,6 +380,10 @@ scsi_alloc_host_cmd_pool(struct Scsi_Host *shost)
pool-slab_flags |= SLAB_CACHE_DMA;
pool-gfp_mask = __GFP_DMA;
}
+
+   if (hostt-cmd_size)
+   hostt-cmd_pool = pool;
+
return pool;
 }
 
@@ -424,8 +428,10 @@ out:
 out_free_slab:
kmem_cache_destroy(pool-cmd_slab);
 out_free_pool:
-   if (hostt-cmd_size)
+   if (hostt-cmd_size) {
scsi_free_host_cmd_pool(pool);
+   hostt-cmd_pool = NULL;
+   }
goto out;
 }
 
@@ -447,8 +453,10 @@ static void scsi_put_host_cmd_pool(struct Scsi_Host *shost)
if (!--pool-users) {
kmem_cache_destroy(pool-cmd_slab);
kmem_cache_destroy(pool-sense_slab);
-   if (hostt-cmd_size)
+   if (hostt-cmd_size) {
scsi_free_host_cmd_pool(pool);
+   hostt-cmd_pool = NULL;
+   }
}
mutex_unlock(host_cmd_pool_mutex);
 }
-- 
1.8.4.5

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

Re: [PATCH V5] Save command pool address of Scsi_Host

[Boaz Harrosh]

On 08/04/2014 02:30 PM, jgr...@suse.com wrote:
 From: Juergen Gross jgr...@suse.com
 
 If a scsi host driver specifies .cmd_len in it's scsi_host_template, a 
 driver's
 private command pool is needed. scsi_find_host_cmd_pool() will locate it, but
 scsi_alloc_host_cmd_pool() isn't saving the pool address in the host template.
 
 This will result in an access error when the host is removed.
 
 Avoid the problem by saving the address of a new allocated command pool where
 it is expected.
 
 Signed-off-by: Juergen Gross jgr...@suse.com
 ---
  drivers/scsi/scsi.c | 12 ++--
  1 file changed, 10 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
 index 88d46fe..b0cef5b 100644
 --- a/drivers/scsi/scsi.c
 +++ b/drivers/scsi/scsi.c
 @@ -380,6 +380,10 @@ scsi_alloc_host_cmd_pool(struct Scsi_Host *shost)
   pool-slab_flags |= SLAB_CACHE_DMA;
   pool-gfp_mask = __GFP_DMA;
   }
 +
 + if (hostt-cmd_size)
 + hostt-cmd_pool = pool;
 +
   return pool;
  }
  
 @@ -424,8 +428,10 @@ out:
  out_free_slab:
   kmem_cache_destroy(pool-cmd_slab);
  out_free_pool:
 - if (hostt-cmd_size)
 + if (hostt-cmd_size) {
   scsi_free_host_cmd_pool(pool);

I disagree I liked the V4 version better. It is much nicer on the review
that we NULL the same one we pass in with no need for context that's outside
of this scope

Just my $0.017
Boaz

 + hostt-cmd_pool = NULL;
 + }
   goto out;
  }
  
 @@ -447,8 +453,10 @@ static void scsi_put_host_cmd_pool(struct Scsi_Host 
 *shost)
   if (!--pool-users) {
   kmem_cache_destroy(pool-cmd_slab);
   kmem_cache_destroy(pool-sense_slab);
 - if (hostt-cmd_size)
 + if (hostt-cmd_size) {
   scsi_free_host_cmd_pool(pool);
 + hostt-cmd_pool = NULL;
 + }
   }
   mutex_unlock(host_cmd_pool_mutex);
  }
 

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

Re: [PATCH V5] Save command pool address of Scsi_Host

[Juergen Gross]

On 08/04/2014 01:49 PM, Boaz Harrosh wrote:

On 08/04/2014 02:30 PM, jgr...@suse.com wrote:

From: Juergen Gross jgr...@suse.com

If a scsi host driver specifies .cmd_len in it's scsi_host_template, a driver's
private command pool is needed. scsi_find_host_cmd_pool() will locate it, but
scsi_alloc_host_cmd_pool() isn't saving the pool address in the host template.

This will result in an access error when the host is removed.

Avoid the problem by saving the address of a new allocated command pool where
it is expected.

Signed-off-by: Juergen Gross jgr...@suse.com
---
  drivers/scsi/scsi.c | 12 ++--
  1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 88d46fe..b0cef5b 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -380,6 +380,10 @@ scsi_alloc_host_cmd_pool(struct Scsi_Host *shost)
pool-slab_flags |= SLAB_CACHE_DMA;
pool-gfp_mask = __GFP_DMA;
}
+
+   if (hostt-cmd_size)
+   hostt-cmd_pool = pool;
+
return pool;
  }

@@ -424,8 +428,10 @@ out:
  out_free_slab:
kmem_cache_destroy(pool-cmd_slab);
  out_free_pool:
-   if (hostt-cmd_size)
+   if (hostt-cmd_size) {
scsi_free_host_cmd_pool(pool);


I disagree I liked the V4 version better. It is much nicer on the review
that we NULL the same one we pass in with no need for context that's outside
of this scope


:-)

I'm fine with either version.

Juergen



Just my $0.017
Boaz


+   hostt-cmd_pool = NULL;
+   }
goto out;
  }

@@ -447,8 +453,10 @@ static void scsi_put_host_cmd_pool(struct Scsi_Host *shost)
if (!--pool-users) {
kmem_cache_destroy(pool-cmd_slab);
kmem_cache_destroy(pool-sense_slab);
-   if (hostt-cmd_size)
+   if (hostt-cmd_size) {
scsi_free_host_cmd_pool(pool);
+   hostt-cmd_pool = NULL;
+   }
}
mutex_unlock(host_cmd_pool_mutex);
  }



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



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