From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 56317C4338F for ; Thu, 19 Aug 2021 14:54:06 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3DABF6113D for ; Thu, 19 Aug 2021 14:54:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240623AbhHSOyl (ORCPT ); Thu, 19 Aug 2021 10:54:41 -0400 Received: from mail.kernel.org ([198.145.29.99]:45864 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238590AbhHSOyk (ORCPT ); Thu, 19 Aug 2021 10:54:40 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 5FAC9610A1; Thu, 19 Aug 2021 14:54:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1629384844; bh=NVt73+uhyzycuJMSmssj+lxbw6moSbp+k7jA2dLxJX8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=0Xb0fhDgt4ezDWxhSKMQAZ7YukIXUtqenZ3/6mMryM+rU8t4Srzqbyz/Vui42QO7N wk2i7ObTpXk6hZnt3J/sRKvURV261jclrUus853dwcSZdjb+WUaBYnApaKTgw1gu7h E3Yo87xlWNx7PGMQ99Cpc+3CLYcWfHlahIPGVoSs= Date: Thu, 19 Aug 2021 16:54:00 +0200 From: Greg Kroah-Hartman To: "Fabio M. De Francesco" Cc: Larry Finger , Phillip Potter , Martin Kaiser , Michael Straube , linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] staging: r8188eu: Remove _enter/_exit_critical_mutex() Message-ID: References: <20210819124955.25540-1-fmdefrancesco@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210819124955.25540-1-fmdefrancesco@gmail.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Aug 19, 2021 at 02:49:55PM +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. > Replace them with the in-kernel API. Ignore return values as it was > in the old code. > > Signed-off-by: Fabio M. De Francesco > --- > > v2: Ignore return values from Mutexes API. > > drivers/staging/r8188eu/core/rtw_mlme_ext.c | 5 +++-- > drivers/staging/r8188eu/hal/usb_ops_linux.c | 5 +++-- > drivers/staging/r8188eu/include/osdep_service.h | 13 ------------- > drivers/staging/r8188eu/os_dep/os_intfs.c | 5 +++-- > 4 files changed, 9 insertions(+), 19 deletions(-) > > diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c > index 5325fe41fbee..9f53cab33333 100644 > --- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c > +++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c > @@ -4359,7 +4359,8 @@ 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); > + if (mutex_lock_interruptible(&pxmitpriv->ack_tx_mutex)) > + ; /*ignore return value */ Ick, no. (not to mention the wrong comment style...) If this really is "criticial", why can it be interrupted? The existing code is such that the code can be interrupted, but if it fails, the lock is not gotten, and the CODE CONTINUES AS IF IT IS OK! So either this is never interruptable (my guess, one almost never needs interruptable locks in a driver) and should just do a normal mutex lock, or the code is totally broken and the locking should be revisited entirely. But a "blind" change like this is not good, let's get it right... thanks, greg k-h