Re: [PATCH] UHCI: add missing memory barriers

[Alan Stern <stern@rowland.harvard.edu>]

On Sat, 17 Dec 2005, Benjamin Herrenschmidt wrote:

> > This patch (as617) adds a couple of memory barriers that Ben H. forgot in
> > his recent suspend/resume fix.
> 
> I didn't think they were necessary but they certainly won't hurt and
> it's not a hot code path...

True.

> >  	pci_write_config_word(to_pci_dev(uhci_dev(uhci)), USBLEGSUP, 0);
> > +	mb();
> 
> Isn't pci config space access always fully synchronous ?

If it is, it's not documented.

Looking at the PCI code, I see that the accesses are protected by a 
spinlock.  Does that guarantee in-order execution of writes to 
configuration space with respect to writes to regular memory?  On all 
platforms?  If yes, then this barrier is not needed.

> > @@ -738,6 +739,7 @@ static int uhci_resume(struct usb_hcd *h
> >  	 * really don't want to keep a stale HCD_FLAG_HW_ACCESSIBLE=0
> >  	 */
> >  	set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
> > +	mb();
> 
> I don't think that one matters much but it won't hurt for sure.

Actually this one only needs to be smp_mb(), although the reasoning is a
bit subtle.  Anyway, as you said, leaving the barriers in certainly won't
hurt anything.

Alan Stern

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