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
| ||
|
Date: Fri, 15 Oct 2021 15:50:20 +0300 From: Dan Carpenter <dan.carpenter@...cle.com> To: "Fabio M. De Francesco" <fmdefrancesco@...il.com> Cc: Larry Finger <Larry.Finger@...inger.net>, Phillip Potter <phil@...lpotter.co.uk>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org Subject: Re: [RFC PATCH] staging: r8188eu: Use completions instead of semaphores On Fri, Oct 15, 2021 at 02:11:41PM +0200, Fabio M. De Francesco wrote: > 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? > The two semaphores used in that function are very connected so I don't think it makes sense to split those up. The others are less connected. regards, dan carpenter
Powered by blists - more mailing lists