Re: [PATCH 09/11] vfs: factor out inode_insert5()

[Miklos Szeredi]

On Mon, Jun 11, 2018 at 05:43:55PM +0100, Al Viro wrote:
> On Mon, Jun 11, 2018 at 01:32:30PM +0200, Miklos Szeredi wrote:
> 
> > Incremental follows.  I think it's cleaner to initialize i_state and i_sb_list
> > up front (hence the use of new_inode()), but could just as well add to sb list
> > afterwards.
> 
> > ---
> > diff --git a/fs/inode.c b/fs/inode.c
> > index 0df41bb77e0f..03c0d7c1296f 100644
> > --- a/fs/inode.c
> > +++ b/fs/inode.c
> > @@ -1098,8 +1098,10 @@ struct inode *iget5_locked(struct super_block *sb, unsigned long hashval,
> >  
> >  		if (new) {
> >  			inode = inode_insert5(new, hashval, test, set, data);
> > -			if (unlikely(inode != new))
> > -				iput(new);
> > +			if (unlikely(inode != new)) {
> > +				inode_sb_list_del(inode);
> > +				destroy_inode(new);
> > +			}
> 
> The thing is, until you put it into the list, it's invisible to everyone other
> than iget5_locked() - no references in any shared data structures.  Which
> outweighs the "it's somewhat irregular in not being on the list" considerably,
> as far as the complexity of analysis goes, especially since there are inodes
> that never get on that list and it's not something exotic - all sockets and
> pipes are that way, for starters.  So IMO that should be dealt with in
> inode_insert5().

Not sure I understand.

We can put inode_sb_list_add() into inode_insert5().  Then what about users of
new_inode() + insert_inode_locked4()?  Those supply an inode that is already on
the sb list.

What about adding to the list after inode_insert5() in the new inode case.

This should be equivalent to what we do currently, AFAICS.

---
diff --git a/fs/inode.c b/fs/inode.c
index 0df41bb77e0f..ad03d4abc600 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1094,12 +1094,14 @@ struct inode *iget5_locked(struct super_block *sb, unsigned long hashval,
 	struct inode *inode = ilookup5(sb, hashval, test, data);
 
 	if (!inode) {
-		struct inode *new = new_inode(sb);
+		struct inode *new = new_inode_pseudo(sb);
 
 		if (new) {
 			inode = inode_insert5(new, hashval, test, set, data);
 			if (unlikely(inode != new))
-				iput(new);
+				destroy_inode(new);
+			else
+				inode_sb_list_add(inode);
 		}
 	}
 	return inode;