| Message ID | 20220110111838.965480-1-yangyingliang@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | [-next] scsi: efct: don't use GFP_KERNEL under spin lock | expand |
On Mon, Jan 10, 2022 at 07:18:38PM +0800, Yang Yingliang wrote: > + spin_lock_irqsave(&node->els_ios_lock, flags); > + > if (els) { > /* initialize fields */ > els->els_retries_remaining = EFC_FC_ELS_DEFAULT_RETRIES; If the els pointer is NULL you will lock the spinlock and disable the interrupts for no reason, maybe you can just protect the list_add_tail()? +spin_lock_irqsave(&node->els_ios_lock, flags); list_add_tail(&els->list_entry, &node->els_ios_list); +spin_unlock_irqrestore(&node->els_ios_lock, flags); Maurizio
Hi, On 2022/1/10 19:19, Maurizio Lombardi wrote: > On Mon, Jan 10, 2022 at 07:18:38PM +0800, Yang Yingliang wrote: >> + spin_lock_irqsave(&node->els_ios_lock, flags); >> + >> if (els) { >> /* initialize fields */ >> els->els_retries_remaining = EFC_FC_ELS_DEFAULT_RETRIES; > If the els pointer is NULL you will lock the spinlock and disable the interrupts > for no reason, maybe you can just protect the list_add_tail()? > > +spin_lock_irqsave(&node->els_ios_lock, flags); > list_add_tail(&els->list_entry, &node->els_ios_list); > +spin_unlock_irqrestore(&node->els_ios_lock, flags); Yes, it's better, I will send a v2 later. Thanks, Yang > > Maurizio > > .
diff --git a/drivers/scsi/elx/libefc/efc_els.c b/drivers/scsi/elx/libefc/efc_els.c index 7bb4f9aad2c8..cec6a2f649b3 100644 --- a/drivers/scsi/elx/libefc/efc_els.c +++ b/drivers/scsi/elx/libefc/efc_els.c @@ -46,18 +46,14 @@ efc_els_io_alloc_size(struct efc_node *node, u32 reqlen, u32 rsplen) efc = node->efc; - spin_lock_irqsave(&node->els_ios_lock, flags); - if (!node->els_io_enabled) { efc_log_err(efc, "els io alloc disabled\n"); - spin_unlock_irqrestore(&node->els_ios_lock, flags); return NULL; } els = mempool_alloc(efc->els_io_pool, GFP_ATOMIC); if (!els) { atomic_add_return(1, &efc->els_io_alloc_failed_count); - spin_unlock_irqrestore(&node->els_ios_lock, flags); return NULL; } @@ -74,7 +70,6 @@ efc_els_io_alloc_size(struct efc_node *node, u32 reqlen, u32 rsplen) &els->io.req.phys, GFP_KERNEL); if (!els->io.req.virt) { mempool_free(els, efc->els_io_pool); - spin_unlock_irqrestore(&node->els_ios_lock, flags); return NULL; } @@ -88,6 +83,8 @@ efc_els_io_alloc_size(struct efc_node *node, u32 reqlen, u32 rsplen) els = NULL; } + spin_lock_irqsave(&node->els_ios_lock, flags); + if (els) { /* initialize fields */ els->els_retries_remaining = EFC_FC_ELS_DEFAULT_RETRIES;
GFP_KERNEL/GFP_DMA can't be used under a spin lock, according the comment of els_ios_lock, it's used to protect els ios list, so we can move down the spin lock to avoid using this flag under the lock. Reported-by: Hulk Robot <hulkci@huawei.com> Fixes: 8f406ef72859 ("scsi: elx: libefc: Extended link Service I/O handling") Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> --- drivers/scsi/elx/libefc/efc_els.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)