Re: [PATCH RESEND] net/cisco: Fix a sleep-in-atomic-context bug in enic_init_affinity_hint()

[Kaige Li <likaige@loongson.cn>]

On 06/25/2020 12:59 AM, Jakub Kicinski wrote:
> On Wed, 24 Jun 2020 06:32:36 +0000 Christian Benvenuti (benve) wrote:
>> We/Cisco will also look into it, hopefully a small code reorg will be sufficient.

Hi, Christian:

I have seen some submissions and codes, and feel that spin_lock is unnecessary in enci_reset.<https://lore.kernel.org/patchwork/project/lkml/list/?submitter=28441>

<https://lore.kernel.org/patchwork/project/lkml/list/?submitter=28441>

Some key submissions are as follows.

Tue Sep 16 00:17:11 2008

git show 01f2e4ead. we can see that spin_lock is just in here:

+       spin_lock(&enic->devcmd_lock);

+       vnic_dev_hang_notify(enic->vdev);

+       spin_unlock(&enic->devcmd_lock);



Sat Aug 17 06:47:40 2013

git show 0b038566c: Add an interface for USNIC to interact with firmware.

Before commit-id: 0b038566c, spin_lock is not used in enic_reset. rtnl_lock() is enough. And 0b038566c add a interface: enic_api_devcmd_proxy_by_index.

Enic_api_devcmd_proxy_by_index is just used in ./drivers/infiniband/hw/usnic/usnic_fwd.c:50, which is added in 2183b990.

+       spin_lock(&enic->enic_api_lock);

         enic_dev_hang_notify(enic);


         enic_dev_set_ig_vlan_rewrite_mode(enic);

         enic_open(enic->netdev);

+       spin_unlock(&enic->enic_api_lock);

By analyzing enic_api_lock, it's mainly used for locking vnic_dev_cmd(vdev, cmd, a0, a1, wait). And enic_reset didn't call to vnic_dev_cmd.

So, I think spin_lock may be deleted in enci_reset. What do you think? Or you have better advice.

Thank you.

> Make sure you enable CONFIG_DEBUG_ATOMIC_SLEEP when you test.