Re: [PATCH 1/3] staging:panel: Rewrite for fixing synchronizationissues

[Dan Carpenter]

On Thu, Jun 28, 2012 at 10:57:29PM +0200, Zoltan Kelemen wrote:

> Partial rewrite of panel driver to make it more robust. The old code contains
> problems potentially leading to kernel oops, deadlocks and memory leaks.
> 
> The patch contains the following changes:
> * Handle asynchronous attach/detach to/from parport.
> * Proper synchronization of multiple opens to same device.
> * Proper disabling of softirqs when locking pprt from user context.
> * Replaced interruptible_sleep_on in keypad_read to avoid race conditions.
> * Changed to del_timer_sync to make sure timer function finishes before
>   unloading module.
> * Added owner to file_operations to avoid module being unloaded with dangling
>   opens.
> * Check status of user-mode memory write in keypad_read.
> * Check for and handle errors in keypad_init.
> * Properly free memory for logical_inputs to avoid memory leak on unload.
> 

Gar...  Don't do this.  :/  Each bullet point here should be a
separate patch.  Please break it apart and resend.

Put the white space changes and the declaration reshuffles in their
own patch.

Do not create lock_pprt() and unlock_pprt() functions.  It doesn't
save any lines of code.  It means that Sparse is unable check for
locking bugs.  It hides information pointlessly so it's considered
bad style.  The comment is nice though and that should go next to
the declaration of the "pprt_lock" spinlock.

regards,
dan carpenter


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/