linux-kernel - Re: [PATCH v8 00/19] staging: r8188eu: shorten and simplify calls chains

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <17555679.WE2PrEGejU@localhost.localdomain>
Date:   Mon, 20 Sep 2021 15:44:53 +0200
From:   "Fabio M. De Francesco" <fmdefrancesco@...il.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Larry Finger <Larry.Finger@...inger.net>,
        Phillip Potter <phil@...lpotter.co.uk>,
        Pavel Skripkin <paskripkin@...il.com>,
        linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org,
        David Laight <david.Laight@...lab.com>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Martin Kaiser <martin@...ser.cx>
Subject: Re: [PATCH v8 00/19] staging: r8188eu: shorten and simplify calls chains

On Monday, September 20, 2021 1:55:42 PM CEST Greg Kroah-Hartman wrote:
> On Mon, Sep 20, 2021 at 01:53:37AM +0200, Fabio M. De Francesco wrote:
> > --- Preface ---
> > 
> > This is v8 of "shorten and simplify calls chain". The first 14 patches
> > have already been applied to staging-testing, so we have been requested
> > to reset the numbering of the remaining patches to 01/19, while 
discarding
> > from this new submission the above-mentioned 14 patches (otherwise we 
would 
> > have submitted a series containing 33 patches).
> > 
> > The following commit message is provided as it was in v7, both for the 
> > purpose of presenting the whole picture to Maintainers, Reviewers, and to 
> > anybody else who may be interested in knowing the entire design and the
> > evolution since v1 to the current v8.
> > 
> > --- Commit message ---
> > 
> > io_ops abstraction is useless in this driver, since there is only one ops
> > registration. Without io_ops we can get rid of indirect calls mess and
> > shorten the calls chain.
> > 
> > Shorten the calls chain of rtw_read8/16/32() down to the actual reads.
> > For this purpose unify the three usb_read8/16/32 into the new
> > usb_read(); make the latter parameterizable with 'size'; embed most of
> > the code of usbctrl_vendorreq() into usb_read() and use in it the new
> > usb_control_msg_recv() API of USB Core.
> > 
> > Shorten the calls chain of rtw_write8/16/32() down to the actual writes.
> > For this purpose unify the four usb_write8/16/32/N() into the new
> > usb_write(); make the latter parameterizable with 'size'; embed most of
> > the code of usbctrl_vendorreq() into usb_write() and use in it the new
> > usb_control_msg_send() API of USB Core.
> > 
> > The code with the modifications was thoroughly tested by Pavel Skripkin
> > using a TP-Link TL-WN722N v2 / v3 [Realtek RTL8188EUS] and by Fabio M.
> > De Francesco using a ASUSTek Computer, Inc. Realtek 8188EUS [USB-N10 
Nano].
> > 
> > --- Changelog ---
> > 
> > v7->v8 (old numbering):
> > 	- 1-14:
> > 		Patches applied to staging-testing, so they are 
dropped
> > 		from the current v8;
> > 
> > 	- 15-19:
> > 		Split into 19 patches. Numbering reset to 01. After 
this
> > 		reset, 15-19/19 become 01-19/19 (so we have a total of 
33
> > 		patches in this series. 
> 
> Better, still needs a bit more work.  I took 2 of these to shorten your
> load a bit :)
>
Thanks, Greg. I've already seen that you took those other 2 from the series. 
We are about half way to destination: 14 + 2 already taken. :)

I've not replied to the other messages of yours because they are easy to 
understand, fix or change commit messages, and resend as v9.

Instead I'm replying to this particular message because I'm not able to see 
where "a bit more work" is needed.

Maybe that you don't like that in paragraph "v7->v8 (old numbering" I only  
said that the old 15-19/19 (4 patches) are now split into 19 without further 
information about each logical group within those 19?

Please notice that, immediately after the above-mentioned section there is 
also the "v7->v8 (new numbering for the old 15-19/19):" new section where 
everything is detailed to a level that we considered sufficient.

For your convenience I copy-paste the lines that immediately follow the "old 
numbering" section:

v7->v8 (new numbering for the old 15-19/19):
        - 1-15:
                Old 15/19 and 16/19 are split into 15 patches (as it is 
                required by Greg Kroah-Hartman) in order to make them
                more easily reviewable and for checking that the logic
                is not broken; no significant changes to the resulting
                code;
        - 16-17:
                Old 17/19 and 18/19 become 16/19 and 17/19: There are 
                no significant changes to the code, with the sole
                exception of moving the acquiring of a mutex before
                the test for "bSurpriseRemoved" being 'true';
        - 18-19:
                Old 19/19 is split into new 18/19 and 19/19. The changes
                are split into a first patch that remove a shared buffer
                and a second that remove the mutex protecting the receiving 
                (in usb_read()) and sending (in usb_write()) of the
                USB control messages. 

If it is not what you require, can you please provide some more hints about 
what you would consider the "perfect" approach to rework this changelog?

It would help a lot, because at the moment I cannot see what I'm missing and 
I've been unable to reach Pavel and ask him if he saw something wrong that I 
can't see.

Thanks,

Fabio
> 
> thanks,
> 
> greg k-h
>