linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg KH <greg@kroah.com>
To: Pavel Skripkin <paskripkin@gmail.com>
Cc: "Fabio M. De Francesco" <fmdefrancesco@gmail.com>,
	Larry Finger <Larry.Finger@lwfinger.net>,
	Phillip Potter <phil@philpotter.co.uk>,
	linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
	David Laight <david.Laight@aculab.com>,
	Dan Carpenter <dan.carpenter@oracle.com>,
	Martin Kaiser <martin@kaiser.cx>
Subject: Re: [PATCH v9 16/16] staging: r8188eu: remove usb_vendor_req_mutex
Date: Thu, 23 Sep 2021 13:07:17 +0200	[thread overview]
Message-ID: <YUxf5TD55Bf2v09v@kroah.com> (raw)
In-Reply-To: <dfa76ffe-2306-01b3-9746-c9aa43c03398@gmail.com>

On Thu, Sep 23, 2021 at 01:12:53PM +0300, Pavel Skripkin wrote:
> On 9/23/21 11:47, Pavel Skripkin wrote:
> > On 9/22/21 16:21, Pavel Skripkin wrote:
> > > On 9/21/21 21:18, Fabio M. De Francesco wrote:
> > > > From: Pavel Skripkin <paskripkin@gmail.com>
> > > > 
> > > > This mutex was used to protect shared buffer for USB requests. Since
> > > > buffer was removed in previous patch we can remove this mutex as well.
> > > > 
> > > > Furthermore, because it was used to serialize the calls to the Core USB
> > > > API, we thoroughly tested the enabling of concurrent firing of USB requests
> > > > without the mutex and found no problems of any kind in common use cases.
> > > > 
> > > > Co-developed-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > > > Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> > > 
> > > Hi, Greg!
> > > 
> > > If all is OK with previous 15 patches, please, do not take this one, it
> > >    causes problems with connection... :)
> > > 
> > > I don't understand what went wrong after v8, but anyway, this one should
> > > not be applied for now, since it's broken
> > > 
> > > 
> > > Thank you
> > > 
> > > 
> > 
> > 
> > Just to be clear: previous 15 patches _are_ tested and do not cause any
> > misbehavior or bugs.
> > 
> > I guess, the stack buffer maybe the problem here, since it's the only
> > change on this side since v8. I didn't have a chance to take a closer
> > look, but I will do it on weekends, I hope :)
> > 
> 
> Oh, I found the problem by just looking at the code with clear mind:
> 
> > -free_dvobj:
> > -	if (status != _SUCCESS && pdvobjpriv) {
> > +	if (pdvobjpriv) {
> >  		usb_set_intfdata(usb_intf, NULL);
> >  		kfree(pdvobjpriv);
> >  		pdvobjpriv = NULL;
> 
> This if should be deleted completely, because we don't want to fail on every
> probe :)
> 
> Sorry for noise... :(
> 
> Greg, can you take first 15 patches, if they look good and then I will send
> fixed version of 16? AFAIU, you are ok with taking part of the series

Please fix up and resend the whole series as our tools work best by
taking the whole thing.

That way I "know" you tested them all :)

thanks,

greg k-h

  reply	other threads:[~2021-09-23 11:07 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-21 18:18 [PATCH v9 00/16] staging: r8188eu: shorten and simplify calls chains Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 01/16] staging: r8188eu: clean up symbols in usbctrl_vendorreq() Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 02/16] staging: r8188eu: reorder declarations " Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 03/16] staging: r8188eu: remove unnecessary test " Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 04/16] staging: r8188eu: reorder comments " Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 05/16] staging: r8188eu: remove a comment from usbctrl_vendorreq() Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 06/16] staging: r8188eu: rename symbols in rtw_read*() and rtw_write*() Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 07/16] staging: r8188eu: remove casts from rtw_{read,write}*() Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 08/16] staging: r8188eu: change the type of a variable in rtw_write16() Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 09/16] staging: r8188eu: remove an buffer from rtw_writeN() Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 10/16] staging: r8188eu: remove a bitwise AND " Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 11/16] staging: r8188eu: change the type of a variable in rtw_read16() Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 12/16] staging: r8188eu: Remove a test from usbctrl_vendorreq() Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 13/16] staging: r8188eu: call new usb_read() from rtw_read{8,16,32}() Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 14/16] staging: r8188eu: call new usb_write() from rtw_write{8,16,32,N}() Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 15/16] staging: r8188eu: remove shared buffer for USB requests Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 16/16] staging: r8188eu: remove usb_vendor_req_mutex Fabio M. De Francesco
2021-09-22 13:21   ` Pavel Skripkin
2021-09-23  8:47     ` Pavel Skripkin
2021-09-23 10:12       ` Pavel Skripkin
2021-09-23 11:07         ` Greg KH [this message]
2021-09-23 11:13           ` Pavel Skripkin

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=YUxf5TD55Bf2v09v@kroah.com \
    --to=greg@kroah.com \
    --cc=Larry.Finger@lwfinger.net \
    --cc=dan.carpenter@oracle.com \
    --cc=david.Laight@aculab.com \
    --cc=fmdefrancesco@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=martin@kaiser.cx \
    --cc=paskripkin@gmail.com \
    --cc=phil@philpotter.co.uk \
    /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).