| Message ID | 1311780065.2356.18.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
> Before even taking locks, we can perform an unlocked test to check if > inode can possibly be in the lists. > > On a 2x4x2 machine, a close(socket()) pair can be 200% faster with these > changes. Great result! Seems simple and obvious to me. -Andi -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jul 27, 2011 at 05:21:05PM +0200, Eric Dumazet wrote: > If I am not mistaken, we can add unlocked checks on the three hot spots. > > After following patch, a close(socket(PF_INET, SOCK_DGRAM, 0)) pair on > my dev machine takes ~3us instead of ~9us. > > Maybe its better to split it in three patches, just let me know. I think three patches would be a lot cleaner. As for safety of the unlocked checks: - inode are either hashed when created or never, so that one looks fine. - same for the sb list. - the writeback list is a bit more dynamic as we move things around quite a bit. But in additon to the inode_wb_list_del call from evict() it only ever gets remove in writeback_single_inode, which for a freeing inode can only be called from the callers of evict(). Btw, I wonder if you should micro-optimize things a bit further by moving the unhashed checks from the deletion functions into the callers and thus save a function call for each of them. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Btw, I wonder if you should micro-optimize things a bit further by > moving the unhashed checks from the deletion functions into the callers > and thus save a function call for each of them. If the caller is in the same file modern gcc is able to do that automatically if you're lucky enough ("partial inlining") I would not uglify the code for it. -Andi
On Wed, Jul 27, 2011 at 10:59:57PM +0200, Andi Kleen wrote: > > Btw, I wonder if you should micro-optimize things a bit further by > > moving the unhashed checks from the deletion functions into the callers > > and thus save a function call for each of them. > > If the caller is in the same file modern gcc is able to do that automatically > if you're lucky enough ("partial inlining") > > I would not uglify the code for it. Depending on how you look at it the code might actually be a tad cleaner. One of called functions is outside of inode.c. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c index 1599aa9..8b90bdb 100644 --- a/fs/fs-writeback.c +++ b/fs/fs-writeback.c @@ -182,11 +182,13 @@ void bdi_start_background_writeback(struct backing_dev_info *bdi) */ void inode_wb_list_del(struct inode *inode) { - struct backing_dev_info *bdi = inode_to_bdi(inode); + if (!list_empty(&inode->i_wb_list)) { + struct backing_dev_info *bdi = inode_to_bdi(inode); - spin_lock(&bdi->wb.list_lock); - list_del_init(&inode->i_wb_list); - spin_unlock(&bdi->wb.list_lock); + spin_lock(&bdi->wb.list_lock); + list_del_init(&inode->i_wb_list); + spin_unlock(&bdi->wb.list_lock); + } } /* diff --git a/fs/inode.c b/fs/inode.c index d0c72ff..796a420 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -338,6 +338,9 @@ static void inode_lru_list_add(struct inode *inode) static void inode_lru_list_del(struct inode *inode) { + if (list_empty(&inode->i_lru)) + return; + spin_lock(&inode->i_sb->s_inode_lru_lock); if (!list_empty(&inode->i_lru)) { list_del_init(&inode->i_lru); @@ -406,6 +409,9 @@ EXPORT_SYMBOL(__insert_inode_hash); */ void remove_inode_hash(struct inode *inode) { + if (inode_unhashed(inode)) + return; + spin_lock(&inode_hash_lock); spin_lock(&inode->i_lock); hlist_del_init(&inode->i_hash);