From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: * X-Spam-Status: No, score=1.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FSL_HELO_FAKE,MAILING_LIST_MULTI,SPF_PASS, USER_AGENT_MUTT autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 93ECEC43381 for ; Fri, 15 Mar 2019 17:38:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5E131218D0 for ; Fri, 15 Mar 2019 17:38:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1552671506; bh=GKCjBMxoHDETdOi7+yf/RyjCK7T7BTtTXqkAGSYJsnc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=JKLhbu12BSin0mFllLhKHH0pUTUoUMObah1Yzxsi7gNuB1BxULWiJr9XjBa867PSj r3k4E+FTzPLXwGu3OqbtNwNqy/8ASR/+gfsoJWtD1sNt51Evveo/0mv9he3qOMhNPc bvw0Yr+j8G22uagAz+uNynqxxg6jAo72tRX1I5LI= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726404AbfCORi0 (ORCPT ); Fri, 15 Mar 2019 13:38:26 -0400 Received: from mail.kernel.org ([198.145.29.99]:58638 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726349AbfCORiZ (ORCPT ); Fri, 15 Mar 2019 13:38:25 -0400 Received: from gmail.com (unknown [104.132.1.77]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id DD260218AC; Fri, 15 Mar 2019 17:38:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1552671505; bh=GKCjBMxoHDETdOi7+yf/RyjCK7T7BTtTXqkAGSYJsnc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=V44oo5tnhz427c/IdcnvmehegK0b/ojkTBXw8vDALO8tKnN5Xk2OCsJ1/P/g6pGwo oMn3dwuTMpocaV+NpN3x7PE5KiUyUwRIiiPVCk4ykpf1xYA/AGPjqy8TDzJ+IsQinR Q1IBzyiV7wsG/yZC4Vv3R2kk0D/NBCdjBAz3GevU= Date: Fri, 15 Mar 2019 10:38:23 -0700 From: Eric Biggers To: Al Viro Cc: "Tobin C. Harding" , linux-fsdevel@vger.kernel.org Subject: Re: dcache locking question Message-ID: <20190315173819.GB77949@gmail.com> References: <20190314225632.GB15813@eros.localdomain> <20190314231939.GA17269@eros.localdomain> <20190315015021.GU2217@ZenIV.linux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190315015021.GU2217@ZenIV.linux.org.uk> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On Fri, Mar 15, 2019 at 01:50:21AM +0000, Al Viro wrote: > > If it fails, we call __lock_parent(). Which > * grabs RCU lock > * drops ->d_lock (now we are not holding ->d_lock > on anything). > * fetches ->d_parent. Note the READ_ONCE() there - > it's *NOT* stable (no ->d_lock held). We can't expect > that ->d_parent won't change or that the reference it used > to contribute to parent's refcount is there anymore; as > the matter of fact, the only thing that prevents outright > _freeing_ of the object 'parent' points to is rcu_read_lock() > and RCU delay between dropping the last reference and > actual freeing of the sucker. rcu_read_lock() is there, > though, which makes it safe to grab ->d_lock on 'parent'. > > That 'parent' might very well have nothing to do with our > dentry by now. We can check if it's equal to its > ->d_parent, though. dentry->d_parent is *NOT* stable > at that point. It might be changing right now. > > However, the first store to dentry->d_parent making it > not equal to parent would have been done under parent->d_lock. > And since we are holding parent->d_lock, we won't miss that > store. We might miss subsequent ones, but if we observe > dentry->d_parent == parent, we know that it's stable. And > if we see dentry->d_parent != parent, we know that dentry > has moved around and we need to retry anyway. Why isn't it necessary to use READ_ONCE(dentry->d_parent) here? if (unlikely(parent != dentry->d_parent)) { Suppose 'parent' is 0xAAAABBBB, and 'dentry->d_parent' is 0xAAAAAAAA and is concurrently changed to 0xBBBBBBBB. d_parent could be read in two parts, 0xAAAA then 0xBBBB, resulting in it appearing that d_parent == 0xAAAABBBB == parent. Yes it won't really be compiled as that in practice, but I thought the point of READ_ONCE() is to *guarantee* it's really done right... - Eric