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  PHC 
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Sun, 17 Oct 2021 10:48:02 +0100
From:   Phillip Potter <>
To:     "Fabio M. De Francesco" <>
Cc:     Larry Finger <>,
        Greg Kroah-Hartman <>,
        Dan Carpenter <>,,
Subject: Re: [PATCH 0/3] staging: r8188eu: use completions and clean

On Sat, Oct 16, 2021 at 07:54:51PM +0200, Fabio M. De Francesco wrote:
> I guess that Dan will disagree with us :) Did you read his last message?
> I hope that he has time to review these patches. He expressed some doubts 
> about splitting the changes in two separate patches. As far as I know, since 
> Dan is a very experienced engineer (I am not even graduated and everything I 
> know of Computer Science is self-taught), I could have been wrong in doing 
> this work the way I did.

I did read it yes, he makes good points, and my motivation is simply
that the patches look fine as they are to me personally :-)

> > given that one semaphore was there for kthread start/stop and the other
> > for queuing. Looks good to me anyway based on what I know of completion
> > variables :-) I assume you've not made the waits killable or
> > interruptible in patch 1 due to the fact they are specifically related
> > to kthread start/stop?
> Good question! :)
> Let me explain how I chose to make one wait killable and the other 
> uninterruptible.
> As far as I know, waiters may spin or sleep while waiting to acquire a lock  
> (see spinlocks or mutexes for instance) or to be awakened (completions and 
> condition variables for instance).
> These were the cases of sleeping waiters. Sleeping can be done in 
> uninterruptible, interruptible / killable, and timed-out states.
> Where I'm 100% sure that the code doesn't require / want to be interrupted 
> for whatever reason I prefer to use uninterruptible variants (and so I did in 
> 1/3).
> When I'm not sure of the requirement above, I prefer to avoid that the 
> process or the entire system hangs while waiting to acquire a mutex or to be 
> awakened by a complete() (and so on).
> Conversely, using interruptible versions without proper checking of return 
> codes and without proper managing of errors may lead to serious bugs.
> Kernel threads (kthreads) are like user processes / threads and are scheduled 
> the same way the former are. One noteworthy difference is that their mm 
> pointer is NULL (they have not an userspace address spaces). However they are 
> still threads that have a PID in userspace and they can be killed by root.
> This is the output of the "ps -ef" command after "modprobe r8188eu":
> localhost:~ # ps -ef | grep RTW
> root      1726     2  0 19:06 ?        00:00:00 [RTW_CMD_THREAD]
> Since the developers who wrote the original code thought that that thread 
> must be interrupted I thought that restricting interruptions to kills was the 
> wiser choice in 2/3. Conversely, I cannot see reasons to interrupt the core 
> part of a driver, so I chose to use an uninterruptible version of 
> wait_for_completion*() in the other parts of the code.
> I warned you that I'm not an engineer, so please double check my argument :)

Sounds good to me, just wanted to know your reasoning.

> > Anyhow:
> > 
> > For whole series:
> > Acked-by: Phillip Potter <>
> Thanks for you ack. I really appreciated it.

You're welcome :-)


Powered by blists - more mailing lists