On 23.02.2018 01:39, David Sterba wrote: > On Thu, Feb 22, 2018 at 12:24:40PM -0700, Liu Bo wrote: >>>>> Not even that far, isize is truncated before calling inode_dio_wait() >>>>> and a memory barrier is set to ensure the correct order, so dio read >>>>> would simply return if it's reading past isize. >>>> >>>> Please, describe concretely which meory barriers pairs with chich in >>>> order to ensure proper visibility. Because I'm of the opinion there are >>>> insufficient memoru barriers to ensure setting READDIO_LOCK is properly >>>> visible in btrfs_direct_IO. Since the latter has no barriers whatsoever. >>> >>> smp_mb() is supposed to be paired, so there is one missing, I agree. >>> >> >> So the missing smp_mb() was there (commit >> 2e60a51e62185cce48758e596ae7cb2da673b58f), but was removed in some >> cleanup I guess. > > Not a cleanup, and not a single commit: > > 38851cc19adbfa1def2b47106d8050a80e0a3673 > Btrfs: serialize unlocked dio reads with truncate > Signed-off-by: Miao Xie <mi...@cn.fujitsu.com> > > at this time the i_dio_counter was still used directly and the barrier > was moved before the if-else block, technically still in the right > before the test_bit. > > dc59215d4f42084ee13654bafe3e5130b146aeb7 > btrfs: remove unnecessary memory barrier in btrfs_direct_IO > Signed-off-by: Nikolay Borisov <nbori...@suse.com> > > ... simply removes the barrier.
Right, I removed this barrier because indeed it wasn't paired with anything in inode_dio_wait. The barrier that is missing here has to order accessing test_bit(READDIO_LOCK) and not ordering the atomic_inc vs inode_dio_wait. > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
