Re: [RFC][PATCH] ensure i_ino uniqueness in filesystems without permanent inode numbers (via idr)

[Jeff Layton <jlayton@redhat.com>]

On Thu, 2006-11-16 at 14:06 +0000, Al Viro wrote:
> 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...
> 

Well, I considered the inode_lock here, but since all of this stuff is
per-sb, I thought s_lock would be a better choice. If that's not
suitable, what do you suggest? A new spinlock to protect the new sb
fields?

> > @@ -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?
> 

I'm not sure what condition I would base this on. That said, I don't
think the impact would be too bad here though. Presumably, those
filesystems that don't use iunique_register will have empty idr hashes
and would return quickly.

> 
> >  	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...

IDR is supposedly quick for this sort of thing though I don't have any
numbers as of yet. Still, getting i_ino uniqueness isn't going to come
for free. There will be some performance impact regardless of what
scheme we use.

-- Jeff