Benjamin Herrenschmidt wrote: > On Thu, 2009-06-18 at 16:30 -0400, Geoff Thorpe wrote: >> I've left the volatile qualifier in the generated API because I didn't >> feel so comfortable changing APIs, but I also added the "memory" clobber >> for all cases - whereas it seems the existing set_bits(), clear_bits(), >> [...] functions didn't declare this... Do you see any issue with having >> the 'volatile' in the prototype as well as the clobber in the asm? >> >> Actually, might as well just respond to the new patch instead... :-) Thx. > > I think the story with the memory clobber is that it depends whether > we consider the functions as ordering accesses or not (ie, can > potentially be used with lock/unlock semantics). > > The general rule is that those who don't return anything don't need > to have those semantics, and thus could only be advertised as clobbering > p[word] -but- there are issues there. For example, despite the > (relatively new) official _lock/_unlock variants, there's still code > that abuses constructs like test_and_set_bit/clear_bit as locks and in > that case, clear bits needs a clobber. > > So I would say at this stage better safe than having to track down > incredibly hard to find bugs, and let's make them all take that clobber.
Well I'm tempted agree because I'm abusing these constructs in exactly the manner you describe. :-) However I didn't know that this was abuse until you mentioned it. Some time ago I noticed that the bitops code was very similar to spinlocks, and so I presumed that a bitops word could act as its own spinlock (ie. rather than spinlocking access to a u32). Now that I look at spinlocks again, I see the presence of those CLEAR_IO_SYNC definitions in the function preambles - is that the distinction I'm abusing? CLEAR_IO_SYNC appears to be undefined except on 64-bit, in which case it's "(get_paca()->io_sync = 0)". W.r.t the _lock/_unlock variants on the bitops side, the "lock" particulars appear to depend on LWSYNC_ON_SMP and ISYNC_ON_SMP, which are "isync" and "lwsync" for all platforms IIUC. So it seems the locking intentions here are different from that of spinlocks? Is there something I can look at that describes what semantics these primitives (are supposed to) guarantee? I may be assuming other things that I shouldn't be ... Cheers, Geoff _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
