[PATCH v2 3/7] staging: unisys: visorinput: ensure proper locking wrt creation & ints

Greg KH gregkh at linuxfoundation.org
Tue Nov 17 22:11:51 UTC 2015


On Mon, Nov 16, 2015 at 03:22:13PM -0500, Benjamin Romer wrote:
> From: Tim Sell <Timothy.Sell at unisys.com>
> 
> Ensure we properly lock between visorinput_channel_interrupt() and
> devdata_create().  We now guarantee that interrupts will be disabled and
> we will be holding lock_visor_dev at the time input_register_device() is
> called (from register_client_keyboard() or register_client_mouse()), after
> which we may quickly find ourselves in visorinput_open(), where we enable
> interrupts, and hence may call visorinput_channel_interrupt().
> 
> Signed-off-by: Tim Sell <Timothy.Sell at unisys.com>
> Signed-off-by: Benjamin Romer <benjamin.romer at unisys.com>
> 
> ---
> v2: the patch was resubmitted.
> ---
>  drivers/staging/unisys/visorinput/visorinput.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/unisys/visorinput/visorinput.c b/drivers/staging/unisys/visorinput/visorinput.c
> index 4620a49..8c22e0f 100644
> --- a/drivers/staging/unisys/visorinput/visorinput.c
> +++ b/drivers/staging/unisys/visorinput/visorinput.c
> @@ -415,6 +415,8 @@ devdata_create(struct visor_device *dev, enum visorinput_device_type devtype)
>  	if (!devdata)
>  		return NULL;
>  	devdata->dev = dev;
> +	init_rwsem(&devdata->lock_visor_dev);
> +	down_write(&devdata->lock_visor_dev);

A rwsem that is only used in write mode?  Not good, just make this a
"normal" lock please, unless you can document that a rwsem is faster.

And for something like an input driver, I _really_ doubt you are ever
going to prove this, or even need such a thing, please fix.

thanks,

greg k-h


More information about the devel mailing list