Messages in this thread | | | Date | Tue, 21 Dec 2004 18:25:14 -0800 | From | Pete Zaitcev <> | Subject | Re: My vision of usbmon |
| |
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 */ 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 - 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/
| |