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;