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)". > > This thread uses interruptible semaphores where instead completions are > more suitable. For this purpose it calls an helper (_rtw_down_sema()) > that returns values that are never checked. It may lead to bugs. > > To address the above-mentioned issues, use two completions variables > instead of semaphores. Use the uninterruptible versions of > wake_for_completion*() because the interruptible / killable versions are > not necessary. > > Tested with "ASUSTek Computer, Inc. Realtek 8188EUS [USB-N10 Nano]". > > This is an RFC patch because I'm not sure that changing this code > from using semaphores to using completions variables is actually required. > After all, the code was working properly with semaphores and, at the same > time, I'm not sure if the Smatch warning about duplicate "up(s)" should > actually be addressed. > > 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@xxxxxxxxx> > --- > Dear Fabio, Sounds like a good approach to me, nice work. I agree with Dan's feedback also - will wait for the final patchset then give it a test for you :-) Apologies I've been a little on the quiet side as of late. Regards, Phil
