Re: [PATCH] iget_locked [1/6]

From: Alexander Viro (viro@math.psu.edu)
Date: Fri May 10 2002 - 15:52:48 EST


On Fri, 10 May 2002, Jan Harkes wrote:

> > > + if (err) {
> > > + destroy_inode(inode);
> > > + return NULL;
> > > + }
> >
> > Please, take that code out of the path - will be cleaner that way.
>
> Ok, a later patch already makes 'set' required, and I was only using the
> failure path in Coda. I'll change this so that set never fails.

I'm not sure that it's a good assumption - just add if (err) goto set_failed;
and take the cleanup there.

> > > destroy_inode: reiserfs_destroy_inode,
> > > read_inode: reiserfs_read_inode,
> > > - read_inode2: reiserfs_read_inode2,
> >
> > Why do we keep ->read_inode() here?
>
> Just in case someone outside of reiser calls 'iget' on a reiserfs inode.
> I guess it's not really necessary to have it around.

Umm... Wait a second. If reiserfs ->read_inode() would work, they wouldn't
need ->read_inode2() in the first place. So any external caller of iget()
is going to have problems anyway, isn't it?
 
> > > Here we simply add an argument to insert_inode_hash. If at some
> > > point a FS specific getattr method is implemented it will be possible to
> > > completely remove all uses of i_ino in the VFS.
> >
> > How about
> >
> > static inline void insert_inode_hash(struct inode *inode)
> > {
> > __insert_inode_hash(inode, inode->i_hash);
>
> Ok, will do that.
>
> Should I create one patch that goes in relative to iget_locked-6, or
> resubmit updated patches for each step? I guess an additional patch is
> the easiest. Or a -6a that replaces the existing -6.

-6a - incremental to -6 would mostly revert the stuff done in -6.

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



This archive was generated by hypermail 2b29 : Tue May 14 2002 - 12:00:15 EST