* [PATCH v2 0/2] libnvdimm/badrange: simplify code
@ 2020-09-19 10:45 Zhen Lei
2020-09-19 10:45 ` [PATCH v2 1/2] libnvdimm/badrange: remove two redundant list_empty() branches Zhen Lei
2020-09-19 10:45 ` [PATCH v2 2/2] libnvdimm/badrange: eliminate a meaningless spinlock operation Zhen Lei
0 siblings, 2 replies; 3+ messages in thread
From: Zhen Lei @ 2020-09-19 10:45 UTC (permalink / raw)
To: Oliver O'Halloran, Dan Williams, Vishal Verma, Dave Jiang,
Ira Weiny, Markus Elfring, linux-nvdimm, linux-kernel
Cc: Zhen Lei, Libin, Kefeng Wang
v1 --> v2:
1. Only the titles and descriptions are modified.
v1:
https://lore.kernel.org/patchwork/cover/1292582/ Patch 1-2
Zhen Lei (2):
libnvdimm/badrange: remove two redundant list_empty() branches
libnvdimm/badrange: eliminate a meaningless spinlock operation
drivers/nvdimm/badrange.c | 32 ++++++++------------------------
1 file changed, 8 insertions(+), 24 deletions(-)
--
1.8.3
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v2 1/2] libnvdimm/badrange: remove two redundant list_empty() branches
2020-09-19 10:45 [PATCH v2 0/2] libnvdimm/badrange: simplify code Zhen Lei
@ 2020-09-19 10:45 ` Zhen Lei
2020-09-19 10:45 ` [PATCH v2 2/2] libnvdimm/badrange: eliminate a meaningless spinlock operation Zhen Lei
1 sibling, 0 replies; 3+ messages in thread
From: Zhen Lei @ 2020-09-19 10:45 UTC (permalink / raw)
To: Oliver O'Halloran, Dan Williams, Vishal Verma, Dave Jiang,
Ira Weiny, Markus Elfring, linux-nvdimm, linux-kernel
Cc: Zhen Lei, Libin, Kefeng Wang
In add_badrange() or badblocks_populate(), the list_empty() branch does
the same things as the code after list_for_each_entry().
The pseudo code is as follows:
1) if (list_empty()) {
do things-A
return Y;
}
2) list_for_each_entry()
do things-B //can only be entered if !list_empty()
3) do things-A
return Y;
It's very clear that, the processing result after deleting 1) is the same
as that before deleting 1).
So delete 1) to simplify code.
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
drivers/nvdimm/badrange.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/drivers/nvdimm/badrange.c b/drivers/nvdimm/badrange.c
index b9eeefa27e3a507..9fdba8c43e8605e 100644
--- a/drivers/nvdimm/badrange.c
+++ b/drivers/nvdimm/badrange.c
@@ -53,13 +53,6 @@ static int add_badrange(struct badrange *badrange, u64 addr, u64 length)
bre_new = kzalloc(sizeof(*bre_new), GFP_KERNEL);
spin_lock(&badrange->lock);
- if (list_empty(&badrange->list)) {
- if (!bre_new)
- return -ENOMEM;
- append_badrange_entry(badrange, bre_new, addr, length);
- return 0;
- }
-
/*
* There is a chance this is a duplicate, check for those first.
* This will be the common case as ARS_STATUS returns all known
@@ -215,9 +208,6 @@ static void badblocks_populate(struct badrange *badrange,
{
struct badrange_entry *bre;
- if (list_empty(&badrange->list))
- return;
-
list_for_each_entry(bre, &badrange->list, list) {
u64 bre_end = bre->start + bre->length - 1;
--
1.8.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH v2 2/2] libnvdimm/badrange: eliminate a meaningless spinlock operation
2020-09-19 10:45 [PATCH v2 0/2] libnvdimm/badrange: simplify code Zhen Lei
2020-09-19 10:45 ` [PATCH v2 1/2] libnvdimm/badrange: remove two redundant list_empty() branches Zhen Lei
@ 2020-09-19 10:45 ` Zhen Lei
1 sibling, 0 replies; 3+ messages in thread
From: Zhen Lei @ 2020-09-19 10:45 UTC (permalink / raw)
To: Oliver O'Halloran, Dan Williams, Vishal Verma, Dave Jiang,
Ira Weiny, Markus Elfring, linux-nvdimm, linux-kernel
Cc: Zhen Lei, Libin, Kefeng Wang
badrange_add() take the lock "badrange->lock", but it's released
immediately in add_badrange(), protect nothing.
The pseudo code is as follows:
In badrange_add():
spin_lock(&badrange->lock); <---------------
rc = add_badrange(badrange, addr, length); |
In add_badrange(): |
//do nothing |
spin_unlock(&badrange->lock); <---------------
bre_new = kzalloc(sizeof(*bre_new), GFP_KERNEL);
spin_lock(&badrange->lock); <--- lock again
This lock/unlock operation is meaningless.
Because the static function add_badrange() is only called by
badrange_add(), so move its content into badrange_add() then delete it.
By the way, move "kfree(bre_new)" out of the lock protection, it really
doesn't need.
Fixes: b3b454f694db ("libnvdimm: fix clear poison locking with spinlock ...")
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
drivers/nvdimm/badrange.c | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/drivers/nvdimm/badrange.c b/drivers/nvdimm/badrange.c
index 9fdba8c43e8605e..7f78b659057902d 100644
--- a/drivers/nvdimm/badrange.c
+++ b/drivers/nvdimm/badrange.c
@@ -45,12 +45,12 @@ static int alloc_and_append_badrange_entry(struct badrange *badrange,
return 0;
}
-static int add_badrange(struct badrange *badrange, u64 addr, u64 length)
+int badrange_add(struct badrange *badrange, u64 addr, u64 length)
{
struct badrange_entry *bre, *bre_new;
- spin_unlock(&badrange->lock);
bre_new = kzalloc(sizeof(*bre_new), GFP_KERNEL);
+
spin_lock(&badrange->lock);
/*
@@ -63,6 +63,7 @@ static int add_badrange(struct badrange *badrange, u64 addr, u64 length)
/* If length has changed, update this list entry */
if (bre->length != length)
bre->length = length;
+ spin_unlock(&badrange->lock);
kfree(bre_new);
return 0;
}
@@ -72,22 +73,15 @@ static int add_badrange(struct badrange *badrange, u64 addr, u64 length)
* as any overlapping ranges will get resolved when the list is consumed
* and converted to badblocks
*/
- if (!bre_new)
+ if (!bre_new) {
+ spin_unlock(&badrange->lock);
return -ENOMEM;
- append_badrange_entry(badrange, bre_new, addr, length);
-
- return 0;
-}
-
-int badrange_add(struct badrange *badrange, u64 addr, u64 length)
-{
- int rc;
+ }
- spin_lock(&badrange->lock);
- rc = add_badrange(badrange, addr, length);
+ append_badrange_entry(badrange, bre_new, addr, length);
spin_unlock(&badrange->lock);
- return rc;
+ return 0;
}
EXPORT_SYMBOL_GPL(badrange_add);
--
1.8.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-09-19 10:46 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-19 10:45 [PATCH v2 0/2] libnvdimm/badrange: simplify code Zhen Lei
2020-09-19 10:45 ` [PATCH v2 1/2] libnvdimm/badrange: remove two redundant list_empty() branches Zhen Lei
2020-09-19 10:45 ` [PATCH v2 2/2] libnvdimm/badrange: eliminate a meaningless spinlock operation Zhen Lei
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).