linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5] staging: r8188eu: Remove _enter/_exit_critical_mutex()
@ 2021-08-28 11:36 Fabio M. De Francesco
  2021-09-02  9:32 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 5+ messages in thread
From: Fabio M. De Francesco @ 2021-08-28 11:36 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Larry Finger, Phillip Potter, Martin Kaiser,
	Michael Straube, Pavel Skripkin, linux-staging, linux-kernel,
	Aakash Hemadri
  Cc: Fabio M. De Francesco

Remove _enter_critical_mutex() and _exit_critical_mutex(). They are
unnecessary wrappers, respectively to mutex_lock_interruptible() and
to mutex_unlock(). They also have an odd interface that takes an unused
argument named pirqL of type unsigned long.
The original code enters the critical section if the mutex API is
interrupted while waiting to acquire the lock; therefore it could lead
to a race condition. Use mutex_lock() because it is uninterruptible and
so avoid that above-mentioned potential race condition.

Tested-by: Pavel Skripkin <paskripkin@gmail.com>
Reviewed-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---

v5: Fix a typo in the subject line. Reported by Aakash Hemadri.

v4: Tested and reviewed by Pavel Skripkin. No changes to the code.

v3: Assume that the original authors don't expect that
mutex_lock_interruptible() can be really interrupted and then lead to 
a potential race condition. Furthermore, Greg Kroah-Hartman makes me
notice that "[] one almost never needs interruptable locks in a driver".
Therefore, replace the calls to mutex_lock_interruptible() with calls to
mutex_lock() since the latter is uninterruptible and avoid race
conditions without the necessity to handle -EINTR errors.

v2: Ignore return values from Mutexes API because the original authors
don't check and manage return errors from mutex_lock_interruptible(). 

 drivers/staging/r8188eu/core/rtw_mlme_ext.c     |  4 ++--
 drivers/staging/r8188eu/hal/usb_ops_linux.c     |  4 ++--
 drivers/staging/r8188eu/include/osdep_service.h | 13 -------------
 drivers/staging/r8188eu/os_dep/os_intfs.c       |  4 ++--
 4 files changed, 6 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 5a472a4954b0..3df02bd0db89 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -4359,7 +4359,7 @@ s32 dump_mgntframe_and_wait_ack(struct adapter *padapter, struct xmit_frame *pmg
 	if (padapter->bSurpriseRemoved || padapter->bDriverStopped)
 		return -1;
 
-	_enter_critical_mutex(&pxmitpriv->ack_tx_mutex, NULL);
+	mutex_lock(&pxmitpriv->ack_tx_mutex);
 	pxmitpriv->ack_tx = true;
 
 	pmgntframe->ack_report = 1;
@@ -4368,7 +4368,7 @@ s32 dump_mgntframe_and_wait_ack(struct adapter *padapter, struct xmit_frame *pmg
 	}
 
 	pxmitpriv->ack_tx = false;
-	_exit_critical_mutex(&pxmitpriv->ack_tx_mutex, NULL);
+	mutex_unlock(&pxmitpriv->ack_tx_mutex);
 
 	return ret;
 }
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 0cf69033c529..065b0d8e030a 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -29,7 +29,7 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
 		goto exit;
 	}
 
-	_enter_critical_mutex(&dvobjpriv->usb_vendor_req_mutex, NULL);
+	mutex_lock(&dvobjpriv->usb_vendor_req_mutex);
 
 	/*  Acquire IO memory for vendorreq */
 	pIo_buf = dvobjpriv->usb_vendor_req_buf;
@@ -92,7 +92,7 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
 			break;
 	}
 release_mutex:
-	_exit_critical_mutex(&dvobjpriv->usb_vendor_req_mutex, NULL);
+	mutex_unlock(&dvobjpriv->usb_vendor_req_mutex);
 exit:
 	return status;
 }
diff --git a/drivers/staging/r8188eu/include/osdep_service.h b/drivers/staging/r8188eu/include/osdep_service.h
index 029aa4e92c9b..bb92b9d74bd7 100644
--- a/drivers/staging/r8188eu/include/osdep_service.h
+++ b/drivers/staging/r8188eu/include/osdep_service.h
@@ -56,19 +56,6 @@ static inline struct list_head *get_list_head(struct __queue *queue)
 	return (&(queue->queue));
 }
 
-static inline int _enter_critical_mutex(struct mutex *pmutex, unsigned long *pirqL)
-{
-	int ret;
-
-	ret = mutex_lock_interruptible(pmutex);
-	return ret;
-}
-
-static inline void _exit_critical_mutex(struct mutex *pmutex, unsigned long *pirqL)
-{
-		mutex_unlock(pmutex);
-}
-
 static inline void rtw_list_delete(struct list_head *plist)
 {
 	list_del_init(plist);
diff --git a/drivers/staging/r8188eu/os_dep/os_intfs.c b/drivers/staging/r8188eu/os_dep/os_intfs.c
index 8d0158f4a45d..6821dcdf1523 100644
--- a/drivers/staging/r8188eu/os_dep/os_intfs.c
+++ b/drivers/staging/r8188eu/os_dep/os_intfs.c
@@ -1062,9 +1062,9 @@ int netdev_open(struct net_device *pnetdev)
 	int ret;
 	struct adapter *padapter = (struct adapter *)rtw_netdev_priv(pnetdev);
 
-	_enter_critical_mutex(padapter->hw_init_mutex, NULL);
+	mutex_lock(padapter->hw_init_mutex);
 	ret = _netdev_open(pnetdev);
-	_exit_critical_mutex(padapter->hw_init_mutex, NULL);
+	mutex_unlock(padapter->hw_init_mutex);
 	return ret;
 }
 
-- 
2.32.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v5] staging: r8188eu: Remove _enter/_exit_critical_mutex()
  2021-08-28 11:36 [PATCH v5] staging: r8188eu: Remove _enter/_exit_critical_mutex() Fabio M. De Francesco
@ 2021-09-02  9:32 ` Greg Kroah-Hartman
  2021-09-03 15:09   ` Pavel Skripkin
  2021-09-04  8:15   ` Fabio M. De Francesco
  0 siblings, 2 replies; 5+ messages in thread
From: Greg Kroah-Hartman @ 2021-09-02  9:32 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: Larry Finger, Phillip Potter, Martin Kaiser, Michael Straube,
	Pavel Skripkin, linux-staging, linux-kernel, Aakash Hemadri

On Sat, Aug 28, 2021 at 01:36:56PM +0200, Fabio M. De Francesco wrote:
> Remove _enter_critical_mutex() and _exit_critical_mutex(). They are
> unnecessary wrappers, respectively to mutex_lock_interruptible() and
> to mutex_unlock(). They also have an odd interface that takes an unused
> argument named pirqL of type unsigned long.
> The original code enters the critical section if the mutex API is
> interrupted while waiting to acquire the lock; therefore it could lead
> to a race condition. Use mutex_lock() because it is uninterruptible and
> so avoid that above-mentioned potential race condition.
> 
> Tested-by: Pavel Skripkin <paskripkin@gmail.com>
> Reviewed-by: Pavel Skripkin <paskripkin@gmail.com>
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> ---
> 
> v5: Fix a typo in the subject line. Reported by Aakash Hemadri.
> 
> v4: Tested and reviewed by Pavel Skripkin. No changes to the code.
> 
> v3: Assume that the original authors don't expect that
> mutex_lock_interruptible() can be really interrupted and then lead to 
> a potential race condition. Furthermore, Greg Kroah-Hartman makes me
> notice that "[] one almost never needs interruptable locks in a driver".
> Therefore, replace the calls to mutex_lock_interruptible() with calls to
> mutex_lock() since the latter is uninterruptible and avoid race
> conditions without the necessity to handle -EINTR errors.

Based on a recent conversation on the linux-usb mailing list, perhaps I
was wrong:
	https://lore.kernel.org/r/20210829015825.GA297712@rowland.harvard.edu

Can you check what happens with your change when you disconnect the
device and these code paths are being called?  That is when you do want
the lock interrupted.

Yes, the logic still seems wrong, but I don't want to see the code now
just lock up entirely with this change as it is a change in how things
work from today.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v5] staging: r8188eu: Remove _enter/_exit_critical_mutex()
  2021-09-02  9:32 ` Greg Kroah-Hartman
@ 2021-09-03 15:09   ` Pavel Skripkin
  2021-09-03 19:11     ` Pavel Skripkin
  2021-09-04  8:15   ` Fabio M. De Francesco
  1 sibling, 1 reply; 5+ messages in thread
From: Pavel Skripkin @ 2021-09-03 15:09 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Fabio M. De Francesco
  Cc: Larry Finger, Phillip Potter, Martin Kaiser, Michael Straube,
	linux-staging, linux-kernel, Aakash Hemadri

On 9/2/21 12:32, Greg Kroah-Hartman wrote:
> On Sat, Aug 28, 2021 at 01:36:56PM +0200, Fabio M. De Francesco wrote:
>> Remove _enter_critical_mutex() and _exit_critical_mutex(). They are
>> unnecessary wrappers, respectively to mutex_lock_interruptible() and
>> to mutex_unlock(). They also have an odd interface that takes an unused
>> argument named pirqL of type unsigned long.
>> The original code enters the critical section if the mutex API is
>> interrupted while waiting to acquire the lock; therefore it could lead
>> to a race condition. Use mutex_lock() because it is uninterruptible and
>> so avoid that above-mentioned potential race condition.
>> 
>> Tested-by: Pavel Skripkin <paskripkin@gmail.com>
>> Reviewed-by: Pavel Skripkin <paskripkin@gmail.com>
>> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
>> ---
>> 
>> v5: Fix a typo in the subject line. Reported by Aakash Hemadri.
>> 
>> v4: Tested and reviewed by Pavel Skripkin. No changes to the code.
>> 
>> v3: Assume that the original authors don't expect that
>> mutex_lock_interruptible() can be really interrupted and then lead to 
>> a potential race condition. Furthermore, Greg Kroah-Hartman makes me
>> notice that "[] one almost never needs interruptable locks in a driver".
>> Therefore, replace the calls to mutex_lock_interruptible() with calls to
>> mutex_lock() since the latter is uninterruptible and avoid race
>> conditions without the necessity to handle -EINTR errors.
> 
> Based on a recent conversation on the linux-usb mailing list, perhaps I
> was wrong:
> 	https://lore.kernel.org/r/20210829015825.GA297712@rowland.harvard.edu
> 
> Can you check what happens with your change when you disconnect the
> device and these code paths are being called?  That is when you do want
> the lock interrupted.
> 
> Yes, the logic still seems wrong, but I don't want to see the code now
> just lock up entirely with this change as it is a change in how things
> work from today.
> 

Hi, Greg!

I've retested this patch with lockdep enabled and I actually hit a 
deadlock. It's really my fault to forgot about lockdep while testing v4, 
I am sorry about the situation.

Actually, the disconnect here is not the problem, the problem was in 
original code. Changing mutex_lock_interruptible to mutex_lock just 
helped to discover it.


The log:

[  252.063305] WARNING: possible recursive locking detected
[  252.063642] 5.14.0+ #9 Tainted: G         C
[  252.063946] --------------------------------------------
[  252.064282] ip/335 is trying to acquire lock:
[  252.064560] ffff888009ebad28 (pmutex){+.+.}-{4:4}, at: 
usbctrl_vendorreq+0xc5/0x4a0 [r8188eu]
[  252.065168]
[  252.065168] but task is already holding lock:
[  252.065536] ffffffffc021b3b8 (pmutex){+.+.}-{4:4}, at: 
netdev_open+0x3a/0x5f [r8188eu]
[  252.066085]
[  252.066085] other info that might help us debug this:
[  252.066494]  Possible unsafe locking scenario:
[  252.066494]
[  252.066866]        CPU0
[  252.067025]        ----
[  252.067184]   lock(pmutex);
[  252.067367]   lock(pmutex);
[  252.067548]
[  252.067548]  *** DEADLOCK ***
[  252.067548]
[  252.067920]  May be due to missing lock nesting notation
[  252.067920]
[  252.068346] 2 locks held by ip/335:
[  252.068570]  #0: ffffffffbda94628 (rtnl_mutex){+.+.}-{4:4}, at: 
rtnetlink_rcv_msg+0x1e0/0x660
[  252.069115]  #1: ffffffffc021b3b8 (pmutex){+.+.}-{4:4}, at: 
netdev_open+0x3a/0x5f [r8188eu]
[  252.069690]
[  252.069690] stack backtrace:
[  252.069968] CPU: 1 PID: 335 Comm: ip Tainted: G         C 
5.14.0+ #9
[  252.071111] Call Trace:
[  252.071273]  dump_stack_lvl+0x45/0x59
[  252.071513]  __lock_acquire.cold+0x1fe/0x31b
[  252.072709]  lock_acquire+0x157/0x3c0
[  252.074445]  __mutex_lock+0xf6/0xc90
[  252.076294]  usbctrl_vendorreq+0xc5/0x4a0 [r8188eu]
[  252.076651]  usb_read8+0x68/0x8f [r8188eu]
[  252.076962]  ? usb_read16+0x8e/0x8e [r8188eu]
[  252.077287]  _rtw_read8+0x2d/0x32 [r8188eu]
[  252.077601]  HalPwrSeqCmdParsing+0x143/0x1de [r8188eu]
[  252.077979]  rtl8188eu_InitPowerOn+0x5a/0xe0 [r8188eu]
[  252.078352]  rtl8188eu_hal_init+0xe7/0x1008 [r8188eu]
[  252.078989]  rtw_hal_init+0x38/0xb5 [r8188eu]
[  252.079317]  _netdev_open+0x282/0x4db [r8188eu]
[  252.079653]  netdev_open+0x42/0x5f [r8188eu]


The problem was here before, but it was race condition, rather than a 
deadlock: netdev_open() locks the mutex, but internally calls usb_read8().

With previous code mutex_lock_interruptible() just fails and execution 
goes forward. It's not correct anyway... Fabio's patch helps to discover 
design bug :)


Again, I am so sorry for not enabling lockdep while testing this first 
time...





With regards,
Pavel Skripkin

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v5] staging: r8188eu: Remove _enter/_exit_critical_mutex()
  2021-09-03 15:09   ` Pavel Skripkin
@ 2021-09-03 19:11     ` Pavel Skripkin
  0 siblings, 0 replies; 5+ messages in thread
From: Pavel Skripkin @ 2021-09-03 19:11 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Fabio M. De Francesco
  Cc: Larry Finger, Phillip Potter, Martin Kaiser, Michael Straube,
	linux-staging, linux-kernel, Aakash Hemadri

On 9/3/21 18:09, Pavel Skripkin wrote:
> On 9/2/21 12:32, Greg Kroah-Hartman wrote:
>> On Sat, Aug 28, 2021 at 01:36:56PM +0200, Fabio M. De Francesco wrote:
>>> Remove _enter_critical_mutex() and _exit_critical_mutex(). They are
>>> unnecessary wrappers, respectively to mutex_lock_interruptible() and
>>> to mutex_unlock(). They also have an odd interface that takes an unused
>>> argument named pirqL of type unsigned long.
>>> The original code enters the critical section if the mutex API is
>>> interrupted while waiting to acquire the lock; therefore it could lead
>>> to a race condition. Use mutex_lock() because it is uninterruptible and
>>> so avoid that above-mentioned potential race condition.
>>> 
>>> Tested-by: Pavel Skripkin <paskripkin@gmail.com>
>>> Reviewed-by: Pavel Skripkin <paskripkin@gmail.com>
>>> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
>>> ---
>>> 
>>> v5: Fix a typo in the subject line. Reported by Aakash Hemadri.
>>> 
>>> v4: Tested and reviewed by Pavel Skripkin. No changes to the code.
>>> 
>>> v3: Assume that the original authors don't expect that
>>> mutex_lock_interruptible() can be really interrupted and then lead to 
>>> a potential race condition. Furthermore, Greg Kroah-Hartman makes me
>>> notice that "[] one almost never needs interruptable locks in a driver".
>>> Therefore, replace the calls to mutex_lock_interruptible() with calls to
>>> mutex_lock() since the latter is uninterruptible and avoid race
>>> conditions without the necessity to handle -EINTR errors.
>> 
>> Based on a recent conversation on the linux-usb mailing list, perhaps I
>> was wrong:
>> 	https://lore.kernel.org/r/20210829015825.GA297712@rowland.harvard.edu
>> 
>> Can you check what happens with your change when you disconnect the
>> device and these code paths are being called?  That is when you do want
>> the lock interrupted.
>> 
>> Yes, the logic still seems wrong, but I don't want to see the code now
>> just lock up entirely with this change as it is a change in how things
>> work from today.
>> 
> 
> Hi, Greg!
> 
> I've retested this patch with lockdep enabled and I actually hit a
> deadlock. It's really my fault to forgot about lockdep while testing v4,
> I am sorry about the situation.
> 
> Actually, the disconnect here is not the problem, the problem was in
> original code. Changing mutex_lock_interruptible to mutex_lock just
> helped to discover it.
> 
> 
> The log:
> 
> [  252.063305] WARNING: possible recursive locking detected
> [  252.063642] 5.14.0+ #9 Tainted: G         C
> [  252.063946] --------------------------------------------
> [  252.064282] ip/335 is trying to acquire lock:
> [  252.064560] ffff888009ebad28 (pmutex){+.+.}-{4:4}, at:
> usbctrl_vendorreq+0xc5/0x4a0 [r8188eu]
> [  252.065168]
> [  252.065168] but task is already holding lock:
> [  252.065536] ffffffffc021b3b8 (pmutex){+.+.}-{4:4}, at:
> netdev_open+0x3a/0x5f [r8188eu]
> [  252.066085]
> [  252.066085] other info that might help us debug this:
> [  252.066494]  Possible unsafe locking scenario:
> [  252.066494]
> [  252.066866]        CPU0
> [  252.067025]        ----
> [  252.067184]   lock(pmutex);
> [  252.067367]   lock(pmutex);
> [  252.067548]
> [  252.067548]  *** DEADLOCK ***
> [  252.067548]
> [  252.067920]  May be due to missing lock nesting notation
> [  252.067920]
> [  252.068346] 2 locks held by ip/335:
> [  252.068570]  #0: ffffffffbda94628 (rtnl_mutex){+.+.}-{4:4}, at:
> rtnetlink_rcv_msg+0x1e0/0x660
> [  252.069115]  #1: ffffffffc021b3b8 (pmutex){+.+.}-{4:4}, at:
> netdev_open+0x3a/0x5f [r8188eu]
> [  252.069690]
> [  252.069690] stack backtrace:
> [  252.069968] CPU: 1 PID: 335 Comm: ip Tainted: G         C
> 5.14.0+ #9
> [  252.071111] Call Trace:
> [  252.071273]  dump_stack_lvl+0x45/0x59
> [  252.071513]  __lock_acquire.cold+0x1fe/0x31b
> [  252.072709]  lock_acquire+0x157/0x3c0
> [  252.074445]  __mutex_lock+0xf6/0xc90
> [  252.076294]  usbctrl_vendorreq+0xc5/0x4a0 [r8188eu]
> [  252.076651]  usb_read8+0x68/0x8f [r8188eu]
> [  252.076962]  ? usb_read16+0x8e/0x8e [r8188eu]
> [  252.077287]  _rtw_read8+0x2d/0x32 [r8188eu]
> [  252.077601]  HalPwrSeqCmdParsing+0x143/0x1de [r8188eu]
> [  252.077979]  rtl8188eu_InitPowerOn+0x5a/0xe0 [r8188eu]
> [  252.078352]  rtl8188eu_hal_init+0xe7/0x1008 [r8188eu]
> [  252.078989]  rtw_hal_init+0x38/0xb5 [r8188eu]
> [  252.079317]  _netdev_open+0x282/0x4db [r8188eu]
> [  252.079653]  netdev_open+0x42/0x5f [r8188eu]
> 
> 

Ok, sorry for noise. It's 100% false positive. Why?

There is no pmutex in this driver. But! *All* mutexes are initialied via 
private _rtw_mutex_init() API, which has a struct mutex *pmutex argument.

So, driver registers all mutexes with the same name to lockdep map. Of 
course, lockdep will complain about _any_ nested locking...


I will prepare a patch to fix this *completely wrong* approach...




With regards,
Pavel Skripkin

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v5] staging: r8188eu: Remove _enter/_exit_critical_mutex()
  2021-09-02  9:32 ` Greg Kroah-Hartman
  2021-09-03 15:09   ` Pavel Skripkin
@ 2021-09-04  8:15   ` Fabio M. De Francesco
  1 sibling, 0 replies; 5+ messages in thread
From: Fabio M. De Francesco @ 2021-09-04  8:15 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Martin Kaiser, Michael Straube,
	Pavel Skripkin, linux-staging, linux-kernel, Aakash Hemadri

On Thursday, September 2, 2021 11:32:07 AM CEST Greg Kroah-Hartman wrote:
> On Sat, Aug 28, 2021 at 01:36:56PM +0200, Fabio M. De Francesco wrote:
> > Remove _enter_critical_mutex() and _exit_critical_mutex(). They are
> > unnecessary wrappers, respectively to mutex_lock_interruptible() and
> > to mutex_unlock(). They also have an odd interface that takes an unused
> > argument named pirqL of type unsigned long.
> > The original code enters the critical section if the mutex API is
> > interrupted while waiting to acquire the lock; therefore it could lead
> > to a race condition. Use mutex_lock() because it is uninterruptible and
> > so avoid that above-mentioned potential race condition.
> > 
> > Tested-by: Pavel Skripkin <paskripkin@gmail.com>
> > Reviewed-by: Pavel Skripkin <paskripkin@gmail.com>
> > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > ---
> > 
> > v5: Fix a typo in the subject line. Reported by Aakash Hemadri.
> > 
> > v4: Tested and reviewed by Pavel Skripkin. No changes to the code.
> > 
> > v3: Assume that the original authors don't expect that
> > mutex_lock_interruptible() can be really interrupted and then lead to 
> > a potential race condition. Furthermore, Greg Kroah-Hartman makes me
> > notice that "[] one almost never needs interruptible locks in a driver".
> > Therefore, replace the calls to mutex_lock_interruptible() with calls to
> > mutex_lock() since the latter is uninterruptible and avoid race
> > conditions without the necessity to handle -EINTR errors.
> 
> Based on a recent conversation on the linux-usb mailing list, perhaps I
> was wrong:
> 	https://lore.kernel.org/r/
20210829015825.GA297712@rowland.harvard.edu
> 
> Can you check what happens with your change when you disconnect the
> device and these code paths are being called?  That is when you do want
> the lock interrupted.
> 
> Yes, the logic still seems wrong, but I don't want to see the code now
> just lock up entirely with this change as it is a change in how things
> work from today.
> 
> thanks,
> 
> greg k-h

Hi Greg,

I guess you've already read the responses from Pavel: He tested the code, 
again and again, and it works properly (connect/disconnect, "ip link show",  
and so on). Furthermore, this patch already has his "Tested-by:" and 
"Reviewed-by:" tags.

Pavel and I agree on the fact that this patch can be applied as-is, however 
we obviously know that it's only up to you.

In the meantime he found and fixed a bad design issue that was revealed by 
using my patch.

Please, let me know if there is anything else I can do.

Thanks,

Fabio




^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-09-04  8:15 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-28 11:36 [PATCH v5] staging: r8188eu: Remove _enter/_exit_critical_mutex() Fabio M. De Francesco
2021-09-02  9:32 ` Greg Kroah-Hartman
2021-09-03 15:09   ` Pavel Skripkin
2021-09-03 19:11     ` Pavel Skripkin
2021-09-04  8:15   ` Fabio M. De Francesco

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