Messages in this thread | | | Date | Tue, 26 Jul 2011 05:03:57 -0400 | From | Christoph Hellwig <> | Subject | Re: [PATCH] vfs: dont chain pipe/anon/socket on superblock s_inodes list |
| |
On Tue, Jul 26, 2011 at 10:21:06AM +0200, Eric Dumazet wrote: > Well, not 'last' contention point, as we still hit remove_inode_hash(),
There should be no ned to put pipe or anon inodes on the inode hash. Probably sockets don't need it either, but I'd need to look at it in detail.
> inode_wb_list_del()
The should never be on the wb list either, doing an unlocked check for actually beeing on the list before taking the lock should help you.
> inode_lru_list_del(),
No real need to keep inodes in the LRU if we only allocate them using new_inode but never look them up either. You might want to try setting .drop_inode to generic_delete_inode for these.
> +struct inode *__new_inode(struct super_block *sb) > +{ > + struct inode *inode = alloc_inode(sb); > + > + if (inode) { > + spin_lock(&inode->i_lock); > + inode->i_state = 0; > + spin_unlock(&inode->i_lock); > + INIT_LIST_HEAD(&inode->i_sb_list); > + } > + return inode; > +}
This needs a much better name like new_inode_pseudo, and a kerneldoc comment explaining when it is safe to use, and the consequences, which appear to me:
- fs may never be unmount - quotas can't work on the filesystem - writeback can't work on the filesystem
> @@ -814,13 +829,9 @@ struct inode *new_inode(struct super_block *sb) > > spin_lock_prefetch(&inode_sb_list_lock); > > - inode = alloc_inode(sb); > - if (inode) { > - spin_lock(&inode->i_lock); > - inode->i_state = 0; > - spin_unlock(&inode->i_lock); > - inode_sb_list_add(inode); > - } > + inode = __new_inode(sb); > + if (inode) > + inode_sb_list_add(inode);
bad indentation.
| |