Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1671729
| From | Jia-Ju Bai <baijiaju1990@163.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH] netxen: Fix a sleep-in-atomic bug in netxen_nic_pci_mem_access_direct |
| Date | 2017-06-21 16:40 +0200 |
| Message-ID | <tUOXx-7i3-55@gated-at.bofh.it> (permalink) |
| References | <tTUVj-56V-11@gated-at.bofh.it> <tUvia-3ak-31@gated-at.bofh.it> <tUH9E-2pq-21@gated-at.bofh.it> <tUHt0-2vO-33@gated-at.bofh.it> <tUOb8-6Jf-3@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On 2017/6/21 21:40, Kalle Valo wrote: > Jia-Ju Bai <baijiaju1990@163.com> writes: > >> On 06/21/2017 02:11 PM, Kalle Valo wrote: >>> David Miller<davem@davemloft.net> writes: >>> >>>> 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... >>> We already tried to explain this to Jia-Ju during review of a wireless >>> patch: >>> >>> https://patchwork.kernel.org/patch/9756585/ >>> >>> Jia-Ju, you should listen to feedback. If you continue submitting random >>> patches like this makes it hard for maintainers to trust your patches >>> anymore. >>> >> Hi, >> >> I am quite sorry for my incorrect patches, and I will listen carefully >> to your advice. In fact, for some bugs and patches which I have >> reported before, I have not received the feedback of them, so I resent >> them a few days ago, including this patch. > Yeah, it is likely that some of your reports will not get any response. > For that I only suggest being persistent and providing more information > about the issue and suggestions how it might be possible to fix it. Also > Dan Carpenter (Cced) might have some suggestions. > > But trying to "fix" it by just silencing the warning without proper > analysis is totally the wrong approach, you do more harm than good. > > What tool do you use to find these issues? Is it publically available? > Hi, Thanks a lot for your advice. And I am very glad to see that you may be interested in my work :) This static tool is written by myself, instead of using or improving existing tools. A reason why I write it is that I have encountered some sleep-in-atomic bugs in my driver development :( . However, due to preliminary implementation, this tool still has some limitations which can produce some false positives or negatives, and it may be not very easy to use. Thus, I am still improving this tool, checking more code and collecting results now. By the way, I apologize again for my incorrect patches of trying to "fix" the detected bugs. In fact, I am very glad to make this tool available to effectively and conveniently check more system code. After I finish the improvements and perform more evaluation, I will make it publicly available. If you have any suggestion or comment on my work, please feel free to contact me :) Thanks, Jia-Ju Bai
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
Re: [PATCH] netxen: Fix a sleep-in-atomic bug in netxen_nic_pci_mem_access_direct David Miller <davem@davemloft.net> - 2017-06-20 19:40 +0200
Re: [PATCH] netxen: Fix a sleep-in-atomic bug in netxen_nic_pci_mem_access_direct Kalle Valo <kvalo@codeaurora.org> - 2017-06-21 08:20 +0200
Re: [PATCH] netxen: Fix a sleep-in-atomic bug in netxen_nic_pci_mem_access_direct Jia-Ju Bai <baijiaju1990@163.com> - 2017-06-21 08:40 +0200
Re: [PATCH] netxen: Fix a sleep-in-atomic bug in netxen_nic_pci_mem_access_direct Bo Yu <tsu.yubo@gmail.com> - 2017-06-21 11:50 +0200
Re: [PATCH] netxen: Fix a sleep-in-atomic bug in netxen_nic_pci_mem_access_direct Kalle Valo <kvalo@codeaurora.org> - 2017-06-21 15:50 +0200
Re: [PATCH] netxen: Fix a sleep-in-atomic bug in netxen_nic_pci_mem_access_direct Jia-Ju Bai <baijiaju1990@163.com> - 2017-06-21 16:40 +0200
Re: [PATCH] netxen: Fix a sleep-in-atomic bug in netxen_nic_pci_mem_access_direct Dan Carpenter <dan.carpenter@oracle.com> - 2017-06-22 08:10 +0200
Re: [PATCH] netxen: Fix a sleep-in-atomic bug in netxen_nic_pci_mem_access_direct Jia-Ju Bai <baijiaju1990@163.com> - 2017-06-22 13:00 +0200
csiph-web