Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755080AbYJOVmT (ORCPT ); Wed, 15 Oct 2008 17:42:19 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752706AbYJOVmL (ORCPT ); Wed, 15 Oct 2008 17:42:11 -0400 Received: from mail.parknet.ad.jp ([210.171.162.6]:39884 "EHLO mail.officemail.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752670AbYJOVmL (ORCPT ); Wed, 15 Oct 2008 17:42:11 -0400 From: OGAWA Hirofumi To: Christoph Hellwig Cc: viro@zeniv.linux.org.uk, akpm@linux-foundation.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/6] vfs: add d_ancestor() References: <20081015195318.GA28349@infradead.org> Date: Thu, 16 Oct 2008 06:42:07 +0900 In-Reply-To: <20081015195318.GA28349@infradead.org> (Christoph Hellwig's message of "Wed, 15 Oct 2008 15:53:18 -0400") Message-ID: <87skqxmt74.fsf@devron.myhome.or.jp> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.0.60 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Anti-Virus: Kaspersky Anti-Virus for MailServers 5.5.10/RELEASE, bases: 24052007 #308098, status: clean Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1610 Lines: 56 Christoph Hellwig writes: > This change looks good, but a slightly more detailed changelog would be > good. tweaked... >> /* >> - * Helper that returns 1 if p1 is a parent of p2, else 0 >> + * Helper that returns the ancestor dentry of p2 which is a child of >> + * p1, if p1 is an ancestor of p2, else NULL. >> */ >> -static int d_isparent(struct dentry *p1, struct dentry *p2) >> +struct dentry *d_ancestor(struct dentry *p1, struct dentry *p2) > > Given that d_ancestor is non-static a kerneldoc comment describing it > instead of a plain one would be good. Yes. >> int is_subdir(struct dentry * new_dentry, struct dentry * old_dentry) >> { >> int result; >> - struct dentry * saved = new_dentry; >> unsigned long seq; >> >> + if (new_dentry == old_dentry) >> + return 1; > > Maybe a comment like in your commit description here would be good? Sure, thanks. > /* > * Need rcu_readlock to protect against the d_parent trashing due to > * d_move. > */ > rcu_read_lock(); > do { > /* for restarting inner loop in case of seq retry */ > seq = read_seqbegin(&rename_lock); > if (d_ancestor(old_dentry, new_dentry)) > result = 1; > else > result = 0; > } while (read_seqretry(&rename_lock, seq)); > rcu_read_unlock(); I'll borrow your version. Thanks. -- OGAWA Hirofumi -- 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/