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
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
