On Mon, 20 Dec 2004 15:25:52 +0100, Oliver Neukum <[EMAIL PROTECTED]> 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 */
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.
-- Pete
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://productguide.itmanagersjournal.com/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
