[PATCH] scsi: ->queue_rq can't sleep

[PATCH] scsi: ->queue_rq can't sleep

[Christoph Hellwig]

The blk-mq ->queue_rq method is always called from process context,
but might have preemption disabled.  This means we still always
have to use GFP_ATOMIC for memory allocations, and thus need to
revert part of commit 3c356bde1 ("scsi: stop passing a gfp_mask
argument down the command setup path").

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reported-by: Sasha Levin <sasha.levin@oracle.com>
---
 drivers/scsi/scsi_lib.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 9ea95dd..6d5c0b8 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -591,7 +591,6 @@ static void scsi_free_sgtable(struct scsi_data_buffer *sdb, bool mq)
 static int scsi_alloc_sgtable(struct scsi_data_buffer *sdb, int nents, bool mq)
 {
 	struct scatterlist *first_chunk = NULL;
-	gfp_t gfp_mask = mq ? GFP_NOIO : GFP_ATOMIC;
 	int ret;
 
 	BUG_ON(!nents);
@@ -606,7 +605,7 @@ static int scsi_alloc_sgtable(struct scsi_data_buffer *sdb, int nents, bool mq)
 	}
 
 	ret = __sg_alloc_table(&sdb->table, nents, SCSI_MAX_SG_SEGMENTS,
-			       first_chunk, gfp_mask, scsi_sg_alloc);
+			       first_chunk, GFP_ATOMIC, scsi_sg_alloc);
 	if (unlikely(ret))
 		scsi_free_sgtable(sdb, mq);
 	return ret;
-- 
1.9.1

Re: [PATCH] scsi: ->queue_rq can't sleep

[Christoph Hellwig]

Any chance to get a review / ack for this one?

On Wed, Jan 07, 2015 at 03:20:53PM +0100, Christoph Hellwig wrote:
> The blk-mq ->queue_rq method is always called from process context,
> but might have preemption disabled.  This means we still always
> have to use GFP_ATOMIC for memory allocations, and thus need to
> revert part of commit 3c356bde1 ("scsi: stop passing a gfp_mask
> argument down the command setup path").
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reported-by: Sasha Levin <sasha.levin@oracle.com>
> ---
>  drivers/scsi/scsi_lib.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 9ea95dd..6d5c0b8 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -591,7 +591,6 @@ static void scsi_free_sgtable(struct scsi_data_buffer *sdb, bool mq)
>  static int scsi_alloc_sgtable(struct scsi_data_buffer *sdb, int nents, bool mq)
>  {
>  	struct scatterlist *first_chunk = NULL;
> -	gfp_t gfp_mask = mq ? GFP_NOIO : GFP_ATOMIC;
>  	int ret;
>  
>  	BUG_ON(!nents);
> @@ -606,7 +605,7 @@ static int scsi_alloc_sgtable(struct scsi_data_buffer *sdb, int nents, bool mq)
>  	}
>  
>  	ret = __sg_alloc_table(&sdb->table, nents, SCSI_MAX_SG_SEGMENTS,
> -			       first_chunk, gfp_mask, scsi_sg_alloc);
> +			       first_chunk, GFP_ATOMIC, scsi_sg_alloc);
>  	if (unlikely(ret))
>  		scsi_free_sgtable(sdb, mq);
>  	return ret;
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
---end quoted text---

Re: [PATCH] scsi: ->queue_rq can't sleep

[Bart Van Assche]

On 01/07/15 15:20, Christoph Hellwig wrote:
> The blk-mq ->queue_rq method is always called from process context,
> but might have preemption disabled.  This means we still always
> have to use GFP_ATOMIC for memory allocations, and thus need to
> revert part of commit 3c356bde1 ("scsi: stop passing a gfp_mask
> argument down the command setup path").
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reported-by: Sasha Levin <sasha.levin@oracle.com>
> ---
>   drivers/scsi/scsi_lib.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 9ea95dd..6d5c0b8 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -591,7 +591,6 @@ static void scsi_free_sgtable(struct scsi_data_buffer *sdb, bool mq)
>   static int scsi_alloc_sgtable(struct scsi_data_buffer *sdb, int nents, bool mq)
>   {
>       struct scatterlist *first_chunk = NULL;
> -     gfp_t gfp_mask = mq ? GFP_NOIO : GFP_ATOMIC;
>       int ret;
>
>       BUG_ON(!nents);
> @@ -606,7 +605,7 @@ static int scsi_alloc_sgtable(struct scsi_data_buffer *sdb, int nents, bool mq)
>       }
>
>       ret = __sg_alloc_table(&sdb->table, nents, SCSI_MAX_SG_SEGMENTS,
> -                            first_chunk, gfp_mask, scsi_sg_alloc);
> +                            first_chunk, GFP_ATOMIC, scsi_sg_alloc);
>       if (unlikely(ret))
>               scsi_free_sgtable(sdb, mq);
>       return ret;
>

Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com>

(note: I have been asked to use my company e-mail address for kernel
related work)

________________________________

PLEASE NOTE: The information contained in this electronic mail message is intended only for the use of the designated recipient(s) named above. If the reader of this message is not the intended recipient, you are hereby notified that you have received this message in error and that any review, dissemination, distribution, or copying of this message is strictly prohibited. If you have received this communication in error, please notify the sender by telephone or e-mail (as shown above) immediately and destroy any and all copies of this message in your possession (whether hard copies or electronically stored copies).

Re: [PATCH] scsi: ->queue_rq can't sleep

[Jens Axboe]

On 01/09/2015 04:51 AM, Christoph Hellwig wrote:
> Any chance to get a review / ack for this one?

You can my reviewed-by, looks fine to me.

-- 
Jens Axboe