On Fri, Jul 27, 2018 at 05:10:39PM +0200, Christoph Hellwig wrote: > On Tue, Jul 24, 2018 at 03:35:23PM -0700, Darrick J. Wong wrote: > > > mutex_unlock contains a barrier, and the sequence is a single register > > > read. There is nothing holding a lock here would help us with. > > > > Which mutex_unlock is that? > > Actually an up_write (on i_lock), but the result is the same. I'm not sure it is - I've lost count of the number of times I've heard the phrase "unlock does not provide a memory barrier, only unlock to lock provides a full memory barrier". As such, I've always treated unlock as a store (release) barrier to keep stores inside the critical section, and lock as the correspending load barrier to keep loads inside the critical section. I think that's irrelevant, anyway, because there is no memory barrier implied until the ILOCK is dropped. i.e. we can change the value before making the extent modification, but there's no guarantee that it's visible to other CPUs until the store memory barrier occurs when the ILOCK is dropped. i.e. after the extent map has actually been changed. And, FWIW, without read-side memory barriers before checking the value, the load can be re-ordered and perhaps even elided by the optimising compiler. IOWs, I can't see any reliable access serialisation being provided by holding the ILOCK while modifying the value. Perhaps this should use WRITE_ONCE/READ_ONCE to ensure the correct memory barriers are used to serialise tthe sequence count against itself rather than the wider inode extent modification process? Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
