Re: [PATCH] iget_locked [1/6]

[Alexander Viro]

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.