David Miller <da...@davemloft.net> writes: > From: Jia-Ju Bai <baijiaju1...@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 <baijiaju1...@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. -- Kalle Valo
