From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nick Piggin Subject: Re: Patch "fs: use RCU read side protection in d_validate" broken Date: Tue, 16 Nov 2010 09:51:08 +1100 Message-ID: <20101115225108.GA3540@amd> References: <20101115051120.GA4092@amd> <20101115210906.GA5964@lst.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Nick Piggin , Linus Torvalds , Al Viro , linux-fsdevel@vger.kernel.org To: Christoph Hellwig Return-path: Received: from ipmail06.adl2.internode.on.net ([150.101.137.129]:6064 "EHLO ipmail06.adl2.internode.on.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758721Ab0KOWvO (ORCPT ); Mon, 15 Nov 2010 17:51:14 -0500 Content-Disposition: inline In-Reply-To: <20101115210906.GA5964@lst.de> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Mon, Nov 15, 2010 at 10:09:06PM +0100, Christoph Hellwig wrote: > On Mon, Nov 15, 2010 at 04:11:20PM +1100, Nick Piggin wrote: > > This patch is totally broken. You can't just dget() a dentry with > > nothing but RCU critical section open. > > The plain dget is indeed wrong as we should at least take d_lock > and check d_count for zero before incrementing it to protect > against shrink_dentry_list. > > I'm not quite sure it really matters as d_validate already has Explain why it doesn't matter. It's an oopsable bug introduced. > and always ad much worse bugs, such as the complete lack of > protection against renames. What are the much worse bugs? What do you mean by rename protection? > Anyway, I'll send a patch to Linus to fix this issue for now. A revert is appropriate. Like I said, there is no need for this patch at all and no justification provided.