Re: [PATCH 08/13] qla2xxx: Fix SRB allocation flag to avoid sleeping in IRQ context

[Giridhar Malavali]

On 1/25/19, 7:55 AM, "linux-scsi-ow...@vger.kernel.org on behalf of Bart Van 
Assche" <linux-scsi-ow...@vger.kernel.org on behalf of bvanass...@acm.org> 
wrote:

    External Email
    
    On Thu, 2019-01-24 at 23:23 -0800, Himanshu Madhani wrote:
    > From: Giridhar Malavali <gmalav...@marvell.com>
    >
    > This patch fixes SRB allocation flag from GFP_KERNEL to GFP_ATOMIC, to
    > prevent sleeping in IRQ context
    >
    > Signed-off-by: Giridhar Malavali <gmalav...@marvell.com>
    > Signed-off-by: Himanshu Madhani <hmadh...@marvell.com>
    > ---
    >  drivers/scsi/qla2xxx/qla_init.c | 2 +-
    >  1 file changed, 1 insertion(+), 1 deletion(-)
    >
    > diff --git a/drivers/scsi/qla2xxx/qla_init.c 
b/drivers/scsi/qla2xxx/qla_init.c
    > index aa72e8316533..3bb4fa97e40a 100644
    > --- a/drivers/scsi/qla2xxx/qla_init.c
    > +++ b/drivers/scsi/qla2xxx/qla_init.c
    > @@ -1829,7 +1829,7 @@ qla24xx_async_abort_cmd(srb_t *cmd_sp, bool wait)
    >       int rval = QLA_FUNCTION_FAILED;
    >
    >       sp = qla2xxx_get_qpair_sp(cmd_sp->vha, cmd_sp->qpair, 
cmd_sp->fcport,
    > -         GFP_KERNEL);
    > +         GFP_ATOMIC);
    >       if (!sp)
    >               goto done;
    
    Is this change necessary because this function can be called from inside a
    timer callback function? Is that callback function qla2x00_sp_timeout()?
    If so, have you considered to modify that function such that it schedules
    the work it has to do? Would that be sufficient to avoid that GFP_KERNEL
    has to be changed into GFP_ATOMIC in qla24xx_async_abort_cmd()?
    
yes, qla2x00_sp_timeout() is the function. Considering that this happens very 
rare I did not consider scheduling this further. 

    Thanks,
    
    Bart.