Re: [PATCH] adding per sb inode list to make invalidate_inodes() faster

[Kirill Korotaev <dev@sw.ru>]

Linus Torvalds wrote:
 > On Fri, 10 Sep 2004, Kirill Korotaev wrote:

 >>>Hmm.. I don't mind the approach per se, but I get very nervous about
 >>>the fact that I don't see any initialization of "inode->i_sb_list".
 >>
 >>inode->i_sb_list is a link list_head, not real list head (real list head
 >>is sb->s_inodes and it's initialized). i.e. it doesn't require
 >>initialization.
 >
 > It _does_ require initialization. And no, there is no difference 
between a
 > "real" head and a entry "link" in the list. They both need to be
 > initialized.
 >
 >>all the operations I perform on i_sb_list are
 >>- list_add(&inode->i_sb_list, ...);
 >
 > This one is ok without initialzing the entry, since it will do so itself.
 >
 >>- list_del(&inode->i_sb_list);
 >
 > This one is NOT ok. If list_del() is ever done on a link entry that 
hasn't
 > been initialized, you crash. If "list_del()" is ever done twice on an
 > entry, you will crash and/or corrupt memory elsewhere.
We never do list_del twice, nor we do list_del on unitialized inodes!

 >>1. struct inode is allocated only in one place!
 >>it's alloc_inode(). Next alloc_inode() is static and is called from 3
 >>places:
 >>new_inode(), get_new_inode() and get_new_inode_fast().
 >>
 >>All 3 above functions do list_add(&inode->i_sb_list, &sb->s_inodes);
 >>i.e. newly allocated inodes are always in super block list.

 > Good. _This_ is what I was after.
 >
 >
 >>2. list_del(&inode->i_sb_list) doesn't leave super block list invalid!
 >
 > No, but it leaves _itself_ invalid. There had better not be anything 
that
 > touches it ever after without an initialization. That wasn't obvious...
ok. But now I hope I proved that it's ok?
no one does list_del() twice and no one does list_del() on unitialized 
inodes.

If you want i_sb_list to be really ALWAYS initialized than we can 
replace list_del with list_del_init and insert INIT_LIST_HEAD(i_sb_list) 
in inode_init_once().
But I don't think it's a good idea.
Moreover, I think that list_del_init() and other initialization 
functions of link list_heads in such places usually only hide the 
problems, not solve them.

Kirill