From: David Miller <davem@davemloft.net>
To: baijiaju1990@163.com
Cc: manish.chopra@cavium.com, rahul.verma@cavium.com,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] netxen: Fix a sleep-in-atomic bug in netxen_nic_pci_mem_access_direct
Date: Tue, 20 Jun 2017 13:35:30 -0400 (EDT) [thread overview]
Message-ID: <20170620.133530.1607963470682255531.davem@davemloft.net> (raw)
In-Reply-To: <1497840533-4894-1-git-send-email-baijiaju1990@163.com>
From: Jia-Ju Bai <baijiaju1990@163.com>
Date: Mon, 19 Jun 2017 10:48:53 +0800
> The driver may sleep under a spin lock, and the function call path is:
> netxen_nic_pci_mem_access_direct (acquire the lock by spin_lock)
> ioremap --> may sleep
>
> To fix it, the lock is released before "ioremap", and the lock is
> acquired again after this function.
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@163.com>
This style of change you are making is really starting to be a
problem.
You can't just drop locks like this, especially without explaining
why it's ok, and why the mutual exclusion this code was trying to
achieve is still going to be OK afterwards.
In fact, I see zero analysis of the locking situation here, why
it was needed in the first place, and why your change is OK in
that context.
Any locking change is delicate, and you must put the greatest of
care and consideration into it.
Just putting "unlock/lock" around the sleeping operation shows a
very low level of consideration for the implications of the change
you are making.
This isn't like making whitespace fixes, sorry...
next prev parent reply other threads:[~2017-06-20 17:35 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-19 2:48 [PATCH] netxen: Fix a sleep-in-atomic bug in netxen_nic_pci_mem_access_direct Jia-Ju Bai
2017-06-20 17:35 ` David Miller [this message]
2017-06-21 6:11 ` Kalle Valo
2017-06-21 6:33 ` Jia-Ju Bai
2017-06-21 13:40 ` Kalle Valo
2017-06-21 14:32 ` Jia-Ju Bai
2017-06-22 6:08 ` Dan Carpenter
2017-06-22 10:52 ` Jia-Ju Bai
2017-06-21 17:44 ` Bo Yu
-- strict thread matches above, loose matches on Subject: below --
2017-05-31 9:21 Jia-Ju Bai
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170620.133530.1607963470682255531.davem@davemloft.net \
--to=davem@davemloft.net \
--cc=baijiaju1990@163.com \
--cc=linux-kernel@vger.kernel.org \
--cc=manish.chopra@cavium.com \
--cc=netdev@vger.kernel.org \
--cc=rahul.verma@cavium.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).