lkml.org 
[lkml]   [2018]   [Jun]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 09/11] vfs: factor out inode_insert5()
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;
\
 
 \ /
  Last update: 2018-06-12 14:39    [W:0.043 / U:3.604 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site