![]() | |||||||||||
Messages in this thread |
Am Mittwoch, 22. Dezember 2004 03:25 schrieb Pete Zaitcev: > On Mon, 20 Dec 2004 15:25:52 +0100, Oliver Neukum <oliver@neukum.org> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ | ||||||||||
| Last update: 2005-03-22 14:08 [from the cache] ©2003-2008 | |||||||||||