[PATCH] Re: Solaris 100K TCP connections, good example? was:[Fwd:

Alexander Viro (viro@math.psu.edu)
Tue, 28 Sep 1999 23:07:18 -0400 (EDT)


On Wed, 29 Sep 1999, Alan Cox wrote:

> > 2. AF_UNIX socket's inode.
> > 3. AF_UNIX socket's struct socket (embedded into inode)
> > 4. AF_UNIX struct's sock.
> > 5. dentry for fs object.
>
> An AF_UNIX file doesnt always have an fs object either..

I'll say... Just what kind of fs object could it possibly have immediately
after socket() or socketpair()?

> > chmoded, etc. - normal ext2 inode. The former has no business being
> > anywhere near icache.
>
> Except for /proc ....

Huh? If you mean /proc/<pid>/fd/<fd> - sorry, no. It has its own inode and
all you have there is follow_link(). Which doesn't touch the inode of
socket - just a dget() on the dentry.

Look: there are very few functions that really scan the hash chains.
That is, iunique() and iget(). Both ignore the inodes with i_sb different
from their argument. Neither is called with sb==NULL, so from their point
of view the socket inodes are ballast. The rest couldn't care less which
chain inode belongs to (some do care whether it belongs to _some_ list,
though). So we have no reason for putting socket and pipe inodes into the
normal hash chains (ones anchored in inode_hashtable).
All we need to do is to modify insert_inode_hash() so that it
would put inodes with ->i_sb==NULL into separate chain. Callers don't need
to know about that - it's purely inode.c business. The patch follows:

--- fs/inode.c.old Tue Sep 28 22:42:31 1999
+++ fs/inode.c Tue Sep 28 22:47:58 1999
@@ -47,6 +47,7 @@
static LIST_HEAD(inode_in_use);
static LIST_HEAD(inode_unused);
static struct list_head inode_hashtable[HASH_SIZE];
+static LIST_HEAD(anon_hash_chain); /* for inodes with NULL i_sb */

/*
* A simple spinlock to protect the list manipulations.
@@ -703,7 +704,9 @@

void insert_inode_hash(struct inode *inode)
{
- struct list_head *head = inode_hashtable + hash(inode->i_sb, inode->i_ino);
+ struct list_head *head = &anon_hash_chain;
+ if (inode->i_sb)
+ head = inode_hashtable + hash(inode->i_sb, inode->i_ino);
spin_lock(&inode_lock);
list_add(&inode->i_hash, head);
spin_unlock(&inode_lock);

It solves all potential problems with icache pollution. The cost being one
(constant) assignment, one comparison and one (not taken) branch per
insert_inode_hash(). It's less than we spend skipping the inode when we
traverse the chain and it's done once. IMO it's a clear win. I can post
the formal proof of correctness if you need it.
Cheers,
Al

-- 
Two of my imaginary friends reproduced once ... with negative results.
				Ben <float@interport.net> in ASR

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/