Re: My vision of usbmon

[Oliver Neukum]

Am Mittwoch, 22. Dezember 2004 03:25 schrieb Pete Zaitcev:
> On Mon, 20 Dec 2004 15:25:52 +0100, Oliver Neukum <oliver@xxxxxxxxxx> wrote:
> 
> > Am Montag, 20. Dezember 2004 08:04 schrieb Pete Zaitcev:
> > > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â memcpy(&mbus->shim_ops, ubus->op, sizeof(struct usb_operations));
> > > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â mbus->shim_ops.submit_urb = mon_submit;
> > > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â mbus->saved_op = ubus->op;
> > > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ubus->op = &mbus->shim_ops;
> > > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ubus->monitored = 1;
> > 
> > I think you need smp_wmb() here to make sure that an irq taken
> > on another CPU sees the manipulations in the correct order.
> 
> Hmm, it seems you are right. I forgot about reordering issues. I relied on
> op being atomic, but if it points at an uninitialized shim, this will end
> badly. How about this?
> 
>                 memcpy(&mbus->shim_ops, ubus->op, sizeof(struct usb_operations));
>                 mbus->shim_ops.submit_urb = mon_submit;
>                 mbus->saved_op = ubus->op;
>                 smp_mb();       /* ubus->op is not protected by spinlocks */
smp_wmb() would do.

>                 ubus->op = &mbus->shim_ops;
>                 ubus->monitored = 1;
> 
> Generally, the type of coding which requires a use of memory barriers in drivers
> is a bug or a latent bug, so I am sorry for the above. It was a sacrifice to
> make usbmon invisible if it's not actively monitoring. Sorry about that.

That is the best way. It's just a little tricky.

	Regards
		Oliver
 
-
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/