Le mardi 26 juillet 2011 à 11:36 +0200, Eric Dumazet a écrit :
> Le mardi 26 juillet 2011 à 05:03 -0400, Christoph Hellwig a écrit :
> > 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.
>
> Yes, it might even help regular inodes ;)
>
> >
> > > 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.
>
> Yes, I'll take a look, thanks.
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.
22us -> 3us, thats a nice patch series ;)
Thanks
[PATCH] vfs: avoid taking locks if inode not in lists
sockets and pipes inodes destruction hits three possibly contended
locks :
system-wide inode_hash_lock in remove_inode_hash()
superblock s_inode_lru_lock in inode_lru_list_del()
bdi wb.list_lock in inode_wb_list_del()
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.
Signed-off-by: Eric Dumazet <eric.dumazet@xxxxxxxxx>
---
fs/fs-writeback.c | 10 ++++++----
fs/inode.c | 6 ++++++
2 files changed, 12 insertions(+), 4 deletions(-)
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);
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
