5:37 p.m.
On Aug 6, 2015, at 5:36 PM, Patrick Farrell wrote:
Perhaps I am confused or better versions of these primitives have
made my understanding outmoded, but isn't it reasonable when a lock is expected to be
held for a very short time to use a spin lock? Something like a single list manipulation
seems like a great candidate for spinning instead of sleeping.
At the least, mutexes are not always better than spinlocks, so a swap without a context
specific explanation of why seems wrong.
Thoughts from others?
Actually what is important about this patch is it highlights operations that make no
sense.
So we add a superblock to the list and then remove a superblock from the list, both under
the lock, and that's it. Huh?
We never traverse the list (at least not under the lock). So I did some digging and yup,
this list is never used other than for
adding and then removing superblocks too.
I did some more digging in the commit history and it was added as part of old bugzilla bug
4699 (in 2004) and the list was
used by the old cache shrinker registered as part of old IO code.
But of course as soon as we moved to the CLIO, this list is no longer used.
Knowing this, we should just drop the list and the spinlock altogether.
________________________________________
From: HPDD-discuss [hpdd-discuss-bounces(a)lists.01.org] on behalf of Shraddha Barke
[shraddha.6596(a)gmail.com]
Sent: Thursday, August 06, 2015 2:37 PM
To: Oleg Drokin; Andreas Dilger; Greg Kroah-Hartman; Al Viro; Tina Johnson; Joe Perches;
HPDD-discuss(a)lists.01.org
Cc: Shraddha Barke
Subject: [HPDD-discuss] [PATCH v2] Staging: lustre: llite: Use a mutex instead of
spinlock
Using a mutex instead of a spinlock results in more flexibility (
i.e it allows to sleep while the lock is held).
Signed-off-by: Shraddha Barke <shraddha.6596(a)gmail.com>
---
Changes in v2:
-Replace DEFINE_SPINLOCK() with DEFINE_MUTEX
drivers/staging/lustre/lustre/llite/llite_lib.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/lustre/lustre/llite/llite_lib.c
b/drivers/staging/lustre/lustre/llite/llite_lib.c
index ab4839c..cd71eaa 100644
--- a/drivers/staging/lustre/lustre/llite/llite_lib.c
+++ b/drivers/staging/lustre/lustre/llite/llite_lib.c
@@ -61,7 +61,7 @@ struct dentry *llite_root;
struct kset *llite_kset;
static LIST_HEAD(ll_super_blocks);
-static DEFINE_SPINLOCK(ll_sb_lock);
+static DEFINE_MUTEX(ll_sb_lock);
#ifndef log2
#define log2(n) ffz(~(n))
@@ -112,9 +112,9 @@ static struct ll_sb_info *ll_init_sbi(struct super_block *sb)
class_uuid_unparse(uuid, &sbi->ll_sb_uuid);
CDEBUG(D_CONFIG, "generated uuid: %s\n", sbi->ll_sb_uuid.uuid);
- spin_lock(&ll_sb_lock);
+ mutex_lock(&ll_sb_lock);
list_add_tail(&sbi->ll_list, &ll_super_blocks);
- spin_unlock(&ll_sb_lock);
+ mutex_unlock(&ll_sb_lock);
sbi->ll_flags |= LL_SBI_VERBOSE;
sbi->ll_flags |= LL_SBI_CHECKSUM;
@@ -145,9 +145,9 @@ static void ll_free_sbi(struct super_block *sb)
struct ll_sb_info *sbi = ll_s2sbi(sb);
if (sbi != NULL) {
- spin_lock(&ll_sb_lock);
+ mutex_lock(&ll_sb_lock);
list_del(&sbi->ll_list);
- spin_unlock(&ll_sb_lock);
+ mutex_unlock(&ll_sb_lock);
kfree(sbi);
}
}
--
2.1.0
_______________________________________________
HPDD-discuss mailing list
HPDD-discuss(a)lists.01.org
https://lists.01.org/mailman/listinfo/hpdd-discuss