Re: [PATCH v2 1/7] staging: unisys: visorinput: use kref ref-counting for device data struct

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Nov 16, 2015 at 03:22:11PM -0500, Benjamin Romer wrote:
> From: Tim Sell <Timothy.Sell@xxxxxxxxxx>
> 
> This is NOT technically required for the code as it stands now, but will
> be needed for subsequent patches.
> 
> Signed-off-by: Tim Sell <Timothy.Sell@xxxxxxxxxx>
> Signed-off-by: Benjamin Romer <benjamin.romer@xxxxxxxxxx>
> 
> ---
> v2: resources are released in the reverse order they were acquired, as per
>     Dan Carpenter's comment.
> ---
>  drivers/staging/unisys/visorinput/visorinput.c | 45 ++++++++++++++++++++------
>  1 file changed, 35 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/staging/unisys/visorinput/visorinput.c b/drivers/staging/unisys/visorinput/visorinput.c
> index 5c16f66..238a132 100644
> --- a/drivers/staging/unisys/visorinput/visorinput.c
> +++ b/drivers/staging/unisys/visorinput/visorinput.c
> @@ -62,6 +62,7 @@ enum visorinput_device_type {
>   * dev_get_drvdata() / dev_set_drvdata() for each struct device.
>   */
>  struct visorinput_devdata {
> +	struct kref kref;
>  	struct visor_device *dev;
>  	struct rw_semaphore lock_visor_dev; /* lock for dev */
>  	struct input_dev *visorinput_dev;
> @@ -346,6 +347,35 @@ register_client_mouse(void *devdata /* opaque on purpose */)
>  	return visorinput_dev;
>  }
>  
> +static void
> +unregister_client_input(struct input_dev *visorinput_dev)
> +{
> +	if (visorinput_dev)
> +		input_unregister_device(visorinput_dev);
> +}
> +
> +static void devdata_release(struct kref *kref)
> +{
> +	struct visorinput_devdata *devdata =
> +		container_of(kref, struct visorinput_devdata, kref);
> +	unregister_client_input(devdata->visorinput_dev);
> +	kfree(devdata);
> +}
> +
> +static struct visorinput_devdata *
> +devdata_get(struct visorinput_devdata *devdata)
> +{
> +	if (devdata)
> +		kref_get(&devdata->kref);
> +	return devdata;
> +}
> +
> +static void devdata_put(struct visorinput_devdata *devdata)
> +{
> +	if (devdata)
> +		kref_put(&devdata->kref, devdata_release);

Where is the lock that protects this from racing?  Please either
document the heck out of this, or use the proper api calls (i.e. the
locked version of the kref release function.)

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux