2022-01-10 11:14:13

by Yang Yingliang

Subject: [PATCH -next] scsi: efct: don't use GFP_KERNEL under spin lock

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 <[email protected]>
Fixes: 8f406ef72859 ("scsi: elx: libefc: Extended link Service I/O handling")
Signed-off-by: Yang Yingliang <[email protected]>
---
drivers/scsi/elx/libefc/efc_els.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)

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;
--
2.25.1

2022-01-10 11:20:18

by Maurizio Lombardi

Subject: Re: [PATCH -next] scsi: efct: don't use GFP_KERNEL under spin lock

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

2022-01-10 12:10:50

by Yang Yingliang

Subject: Re: [PATCH -next] scsi: efct: don't use GFP_KERNEL under spin lock

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
>
> .