From mboxrd@z Thu Jan  1 00:00:00 1970
From: Christoph Hellwig <hch@infradead.org>
Subject: Re: [PATCH 3/4] XFS: Return case-insensitive match for dentry cache
Date: Tue, 20 May 2008 14:23:15 -0400
Message-ID: <20080520182315.GA8456@infradead.org>
References: <20080513075749.477238845@chook.melbourne.sgi.com> <20080513080152.911303131@chook.melbourne.sgi.com> <20080513085724.GC21919@infradead.org> <op.ua4wa7t03jf8g2@pc-bnaujok.melbourne.sgi.com> <20080515045700.GA4328@infradead.org> <op.ua6ji4r93jf8g2@pc-bnaujok.melbourne.sgi.com> <DCB15FFF-F942-47BD-B8FB-38AADC24B9D6@cam.ac.uk> <op.ubfkzvjx3jf8g2@pc-bnaujok.melbourne.sgi.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Anton Altaparmakov <aia21@cam.ac.uk>,
	Christoph Hellwig <hch@infradead.org>, xfs@oss.sgi.com,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
To: Barry Naujok <bnaujok@sgi.com>
Return-path: <linux-fsdevel-owner@vger.kernel.org>
Received: from bombadil.infradead.org ([18.85.46.34]:47726 "EHLO
	bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1753852AbYETSXQ (ORCPT
	<rfc822;linux-fsdevel@vger.kernel.org>);
	Tue, 20 May 2008 14:23:16 -0400
Content-Disposition: inline
In-Reply-To: <op.ubfkzvjx3jf8g2@pc-bnaujok.melbourne.sgi.com>
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On Tue, May 20, 2008 at 12:24:57PM +1000, Barry Naujok wrote:
> +/**
> + * d_drop_neg_children - drop negative child dentries
> + * @parent: parent dentry
> + *
> + * Searches the children of the @parent dentry for negative dentries and
> + * drops them as they are found.
> + *
> + * This is primarily useful for case-insensitive filesystems to drop these
> + * entries when a new entry is created in the parent. The new entry must
> + * be instantiated before calling this function.
> + */
> +
> +void d_drop_neg_children(struct dentry *parent)

please spell out the negative :)  also no empty line between the
kerneldoc sand the actual function please.

> +{
> +	struct dentry *dentry;
> +
> +	spin_lock(&dcache_lock);
> +	list_for_each_entry(dentry, &parent->d_subdirs, d_u.d_child) {
> +		if (!dentry->d_inode) {
> +			spin_lock(&dentry->d_lock);
> +			__d_drop(dentry);
> +			spin_unlock(&dentry->d_lock);
> +			cond_resched_lock(&dcache_lock);

The cond_resched_lock here is not safe here, because the pointer you
are going to dereference in list_for_each_entry might not be valid
anymore.  This should look more like:

void d_drop_negative_children(struct dentry *parent)
{
	struct dentry *dentry;

 again:
	spin_lock(&dcache_lock);
	list_for_each_entry(dentry, &parent->d_subdirs, d_u.d_child) {
		if !dentry->d_inode)
			continue;

		spin_lock(&dentry->d_lock);
		__d_drop(dentry);
		spin_unlock(&dentry->d_lock);

		if (need_resched()) {
			spin_unlock(&dcache_lock);
			cond_resched();
			goto again;
		}
	}
	spin_unlock(&dcache_lock);
}


Btw, any reason you haven't commited patches 1 and 2 yet?  Also maybe
splitting 3 and 4 differently with one patch for the two new functions
in dcache.c and one for the full XFS ascii CI support might be best.