From mboxrd@z Thu Jan  1 00:00:00 1970
From: Al Viro <viro@ftp.linux.org.uk>
Subject: Re: [RFC][PATCH] ensure i_ino uniqueness in filesystems without permanent inode numbers (via idr)
Date: Thu, 16 Nov 2006 14:06:22 +0000
Message-ID: <20061116140621.GG29920@ftp.linux.org.uk>
References: <1163535728.15846.21.camel@dantu.rdu.redhat.com> <20061114202625.GY29920@ftp.linux.org.uk> <1163608958.7571.8.camel@tleilax.poochiereds.net>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: linux-fsdevel@vger.kernel.org
Return-path: <linux-fsdevel-owner@vger.kernel.org>
Received: from zeniv.linux.org.uk ([195.92.253.2]:24285 "EHLO
	ZenIV.linux.org.uk") by vger.kernel.org with ESMTP id S1424080AbWKPOGW
	(ORCPT <rfc822;linux-fsdevel@vger.kernel.org>);
	Thu, 16 Nov 2006 09:06:22 -0500
To: Jeff Layton <jlayton@redhat.com>
Content-Disposition: inline
In-Reply-To: <1163608958.7571.8.camel@tleilax.poochiereds.net>
Sender: linux-fsdevel-owner@vger.kernel.org
List-Id: linux-fsdevel.vger.kernel.org

On Wed, Nov 15, 2006 at 11:42:38AM -0500, Jeff Layton wrote:
> +{
> +	int rv;
> +
> +	rv = idr_pre_get(&inode->i_sb->s_inode_ids, GFP_KERNEL);
> +	if (! rv)
> +		return -ENOMEM;
> +
> +	lock_super(inode->i_sb);

?!#!@#!???

Please, use something saner.  Use of lock_super() for anything generic
is wrong; using it for something that'd better be fast is even dumber...

> @@ -1025,6 +1055,7 @@ void generic_delete_inode(struct inode *
>  	spin_lock(&inode_lock);
>  	hlist_del_init(&inode->i_hash);
>  	spin_unlock(&inode_lock);
> +	iunique_unregister(inode);

Unconditional?  Hitting every fs out there?  With that kind of locking?

>  	wake_up_inode(inode);
>  	BUG_ON(inode->i_state != I_CLEAR);
>  	destroy_inode(inode);
> @@ -1057,6 +1088,7 @@ static void generic_forget_inode(struct 
>  	inode->i_state |= I_FREEING;
>  	inodes_stat.nr_inodes--;
>  	spin_unlock(&inode_lock);
> +	iunique_unregister(inode);

Ditto.

>  	if (inode->i_data.nrpages)
>  		truncate_inode_pages(&inode->i_data, 0);
>  	clear_inode(inode);
> diff --git a/fs/pipe.c b/fs/pipe.c
> index b1626f2..d74ae65 100644
> --- a/fs/pipe.c
> +++ b/fs/pipe.c
> @@ -845,6 +845,9 @@ static struct inode * get_pipe_inode(voi
>  	if (!inode)
>  		goto fail_inode;
>  
> +	if (iunique_register(inode, 0))
> +		goto fail_iput;
> +

Humm...  I wonder what the overhead of that is going to be.  There
easily can be shitloads of pipes on the box, with all sorts of
lifetimes.  And we'd better be fast on that codepath...