Re: [PATCH] scsi_debug: fix sleep in invalid context

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Mon, 2016-05-30 at 14:19 -0400, Douglas Gilbert wrote:
> In this post: http://www.spinics.net/lists/linux-scsi/msg97124.html
> the author shows some kernel infrastructure complaining about a
> sleep in an invalid context. For calls to fetch memory when
> processing SCSI commands, reviewers often propose non GFP_ATOMIC
> variants; reality dictates otherwise. Fix memory allocation for
> response to REPORT LUNS command.
> 
> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
> ---
>  drivers/scsi/scsi_debug.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 0f9ba41..b85c5dc 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -3331,13 +3331,12 @@ static int resp_report_luns(struct scsi_cmnd
> *scp,
>  	tlun_cnt = lun_cnt + wlun_cnt;
>  
>  	rlen = (tlun_cnt * sizeof(struct scsi_lun)) + 8;
> -	arr = vmalloc(rlen);
> +	arr = kzalloc(rlen, GFP_ATOMIC);
>  	if (!arr) {
>  		mk_sense_buffer(scp, ILLEGAL_REQUEST,
> INSUFF_RES_ASC,
>  				INSUFF_RES_ASCQ);
>  		return check_condition_result;
>  	}
> -	memset(arr, 0, rlen);
>  	pr_debug("select_report %d luns = %d wluns = %d no_lun0
> %d\n",
>  		 select_report, lun_cnt, wlun_cnt, sdebug_no_lun_0);
>  
> @@ -3355,7 +3354,7 @@ static int resp_report_luns(struct scsi_cmnd
> *scp,
>  	put_unaligned_be32(rlen - 8, &arr[0]);
>  
>  	res = fill_from_dev_buffer(scp, arr, rlen);
> -	vfree(arr);
> +	kfree(arr);

This might fix the immediate warning, but won't it demand huge
contiguous memory chunks in high lun configurations and thus fail
randomly?  Report luns is important to us because if that fails the
target won't attach.

What about vmalloc'ing enough space at configuration time, when you do
have process context, and simply reusing the already allocated buffer
in this routine?  If you want to be clever, you could do a single
vmalloc for the biggest LUN size you have and reuse that buffer for
every report lun command with suitable locking ... we tend to fire off
report luns sequentially at start of day, so it's not like they have
huge performance or concurrency requirements.

James