Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751868AbbGaWwl (ORCPT ); Fri, 31 Jul 2015 18:52:41 -0400 Received: from mail-io0-f175.google.com ([209.85.223.175]:36808 "EHLO mail-io0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751029AbbGaWwj (ORCPT ); Fri, 31 Jul 2015 18:52:39 -0400 MIME-Version: 1.0 In-Reply-To: <20150731205036.GA3752@nautica> References: <20150731205036.GA3752@nautica> Date: Fri, 31 Jul 2015 15:52:38 -0700 X-Google-Sender-Auth: N5AkraZNGkULybxUW4BoiBw1Ip8 Message-ID: Subject: Re: v4.2-rc dcache regression, probably 75a6f82a0d10 From: Linus Torvalds To: Dominique Martinet Cc: Hugh Dickins , "J. Bruce Fields" , Dominique Martinet , Al Viro , Linux Kernel Mailing List , linux-fsdevel , David Howells Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2666 Lines: 61 On Fri, Jul 31, 2015 at 1:50 PM, Dominique Martinet wrote: > > It's probably an old race that was very hard to hit because of cache > coherency. > Basically, before the wmb/rmb, the dentry was always updated closely to > its flags, so the other CPU would "usually" get both updates at the same > time; the barriers make it so the updates are split and it's possible to > get it, and would explain why I could pick 4bf46a2726 as "the one" I doubt that's it. wmb/rmb is actually a no-op on x86, and only affects instruction scheduling. So yes, timing could be affected, but it's such a _tiny_ effect that I don't really believe it's an issue. I'd be more suspicious about other effects. For example, iot's not at all obvious that the commit in question just changes the order of the flags/inode field accesses, there are potentialy bigger changes there. For example, this part (in __d_obtain_alias): - tmp->d_inode = inode; - tmp->d_flags |= add_flags; + __d_set_inode_and_type(tmp, inode, add_flags); looks a bit off, because it *used* to just add those flags, but now, through __d_set_inode_and_type, it does + dentry->d_inode = inode; + smp_wmb(); + flags = READ_ONCE(dentry->d_flags); + flags &= ~(DCACHE_ENTRY_TYPE | DCACHE_FALLTHRU); + flags |= type_flags; + WRITE_ONCE(dentry->d_flags, flags); so it clears DCACHE_ENTRY_TYPE | DCACHE_FALLTHRU. Is that correct? Maybe, I haven't checked. And maybe it's a big bad bug. Regardless, it sure as hell isn't just changing the order of the access to those fields. That "DCACHE_ENTRY_TYPE | DCACHE_FALLTHRU" clearing came from __d_instantiate(), but now it hits __d_obtain_alias too. There may be other changes like that for all I know. I didn't look more closely. But I'd blame changes like that rather than any timing by rmb/wmb. (Side note: the instruction scheduling changes can be big - especially together with the changes to use READ_ONCE/WRITE_ONCE, it basically means that gcc won't be using r-m-w instructions etc to set flags, so I'm not saying rmb/wmb is necessarily a no-op in general, but it's definitely generally not a hugely noticeable thing). Added DavidH to the cc list, since it's his commit. But Dominique, it would probably be a good idea for you to try to double-check that that commit really is what matters for you and causes problems. Linus -- 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/