Re: [PATCH]smp race in ipw driver

On Thu, Mar 29, 2007 at 01:59:58PM +0200, Oliver Neukum wrote:
> Hi,
> 
> another one in my traversal through the serial drivers.
> 
> This here is beautiful in a haiku sort of way.
> You must read the urb's status before you declare it non busy or
> you may read the next urb's status, not your own, and you must guard
> against write reordering.
> 
> 	Regards
> 		Oliver
> Signed-off-by: Oliver Neukum <[email protected]> 
> -- 
> --- a/drivers/usb/serial/ipw.c	2007-03-29 13:50:52.000000000 +0200
> +++ b/drivers/usb/serial/ipw.c	2007-03-29 13:53:46.000000000 +0200
> @@ -369,13 +369,16 @@
>  static void ipw_write_bulk_callback(struct urb *urb)
>  {
>  	struct usb_serial_port *port = urb->context;
> +	int status;
>  
>  	dbg("%s", __FUNCTION__);
>  
> +	status = urb->status;
> +	smp_mb(); /* status must be read before urb is declared unbusy */

Why is this necessary?  'status' should always be safe to read while
within the urb callback.  If not, then we need to add this to _every_ urb
callback?

Why is this necessary?  How can that code be "wrong"?

thanks,

greg k-h

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel