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]
Date:   Fri, 15 Oct 2021 14:11:41 +0200
From:   "Fabio M. De Francesco" <fmdefrancesco@...il.com>
To:     Dan Carpenter <dan.carpenter@...cle.com>,
        Larry Finger <Larry.Finger@...inger.net>,
        Phillip Potter <phil@...lpotter.co.uk>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] staging: r8188eu: Use completions instead of semaphores

On Friday, October 15, 2021 1:37:15 PM CEST Dan Carpenter wrote:
> On Fri, Oct 15, 2021 at 01:02:38PM +0200, Fabio M. De Francesco wrote:
> > rtw_cmd_thread() "up(s)" a semaphore twice, first to notify callers when
> > its execution is started and then to notify when it is about to end.
> > 
> > It makes the same semaphore go "up" twice in the same thread. This
> > construct makes Smatch to warn of duplicate "up(s)".
> >
> > [...]
> >
> > I'm waiting for Maintainers and other Reviewers to say if this patch is
> > actually needed and, if so, also for suggestions about how to improve
> > it. In particular I'm interested to know what they think of using the
> > uninterruptible version of wait_for_completion*().
> > 
> > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@...il.com>
> 
> This is basically what Arnd did to rtl8723bs in commit:
> 
> commit 09a8ea34cf431bfb77159197e46753d101c528c5
> Author: Arnd Bergmann <arnd@...db.de>
> Date:   Mon Dec 10 22:40:30 2018 +0100
> 
>     staging: rtl8723bs: change semaphores to completions
> 
> But there are some differences.  His patch is a little bit cleaner
> because it gets rid of "pcmdpriv->cmd_queue_sema".  Could you basically
> just ports Arnd's patch for this driver?
> 
> His patch goes quite a bit further as well, and change some other
> semaphors but we could do it piece meal and just change the
> rtw_cmd_thread() related ones.
> 
> regards,
> dan carpenter
> 
Hi Dan,

Thanks for your review. 

I wasn't aware of Arnd's patch. If I were I would have sent a "normal" patch.

Beyond this, I noticed that other semaphore (pcmdpriv->cmd_queue_sema) but, 
since I was not 100% sure that my changes would be accepted, I decided to 
leave it as-is for now and wait for reviews like yours.

Now that I know that this changes are welcome I'll also make the other 
changes. 

I guess that I have to change one semaphore per patch and make a series. 
However, now I see that Arnd's patch makes all the necessary changes in a 
single patch. What is the correct approach? Is one patch per semaphore 
preferred or one big patch for all of those that need to be changed?

Again, thank you very much.

Regards,

Fabio M. De Francesco


Powered by blists - more mailing lists