On Wed, Mar 6, 2019 at 4:48 PM Michael Ellerman <mpe@xxxxxxxxxxxxxx> wrote: > > It's a bit hard to grep for, but I did find one case: > > static void netxen_nic_io_write_128M(struct netxen_adapter *adapter, > void __iomem *addr, u32 data) > { > read_lock(&adapter->ahw.crb_lock); > writel(data, addr); > read_unlock(&adapter->ahw.crb_lock); > } > > It looks like that driver is using the rwlock to exclude cases that can > just do a readl()/writel() (readers) vs another case that has to reconfigure a > window or something, before doing readl()/writel() and then configuring > the window back. So that seems like a valid use for a rwlock. Oh, it's actually fairly sane: the IO itself is apparently windowed on that hardware, and the *common* case is that you'd access "window 1". So if everybody accesses window 1, they can all work in parallel - the read case. But if somebody needs to access any of the other special IO windows, they need to take the write lock, then change the window pointer to the window they want to access, do the access, and then set it back to the default "window 1". So yes. That driver very much relies on exclusion of the IO through an rwlock. I'm guessing nobody uses that hardware on Power? Or maybe the "window 1 is common" is *so* common that the other cases basically never happen and don't really end up ever causing problems? [ Time passes, I look at it ] Actually, the driver probably works on Power, because *setting* the window isn't just a write to the window register, it's always serialized by a read _from_ the window register to verify that the write "took". Apparently the hardware itself really needs that "don't do accesses to the window before I've settled". And that would basically serialize the only operation that really needs serialization, so in the case of _that_ driver, it all looks safe. Even if it's partly by luck. > > Perhaps more importantly, what about sleeping locks? When they > > actually *block*, they get the barrier thanks to the scheduler, but > > you can have a nice non-contended sequence that never does that. > > Yeah. > > The mutex unlock fast path is just: Yup. Both lock/unlock have fast paths that should be just trivial atomic sequences. But the good news is that *usually* device IO is protected by a spinlock, since you almost always have interrupt serialization needs too whenever you have any sequence of MMIO that isn't just some "write single word to start the engine". So the "use mutex to serialize IO" may be fairly unusual. Linus