* [PATCH 0/4] clean up some functions in nvdimm/badrange.c and namespace_devs.c
@ 2020-08-20 14:30 Zhen Lei
2020-08-20 14:30 ` [PATCH 1/4] libnvdimm: remove redundant list_empty() check in badrange.c Zhen Lei
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Zhen Lei @ 2020-08-20 14:30 UTC (permalink / raw)
To: Oliver O'Halloran, Dan Williams, Vishal Verma, Dave Jiang,
Ira Weiny, linux-nvdimm, linux-kernel
Cc: Zhen Lei
When I learned the code of drivers/nvdimm, I found some places can be improved.
Zhen Lei (4):
libnvdimm: remove redundant list_empty() check in badrange.c
libnvdimm: eliminate a meaningless spinlock operation
libnvdimm: eliminate two unnecessary zero initializations in
badrange.c
libnvdimm: avoid unnecessary judgments in nvdimm_namespace_disk_name()
drivers/nvdimm/badrange.c | 36 ++++++++++--------------------------
drivers/nvdimm/namespace_devs.c | 12 +++++-------
2 files changed, 15 insertions(+), 33 deletions(-)
--
1.8.3
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/4] libnvdimm: remove redundant list_empty() check in badrange.c
2020-08-20 14:30 [PATCH 0/4] clean up some functions in nvdimm/badrange.c and namespace_devs.c Zhen Lei
@ 2020-08-20 14:30 ` Zhen Lei
2020-08-20 14:30 ` [PATCH 2/4] libnvdimm: eliminate a meaningless spinlock operation Zhen Lei
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Zhen Lei @ 2020-08-20 14:30 UTC (permalink / raw)
To: Oliver O'Halloran, Dan Williams, Vishal Verma, Dave Jiang,
Ira Weiny, linux-nvdimm, linux-kernel
Cc: Zhen Lei
In add_badrange() or badblocks_populate(), the list_empty() branch does
the same things as the code after list_for_each_entry(). And the
list_for_each_entry() will do noting when list_empty().
So the list_empty() branch can be removed without functional change, let
the code after list_for_each_entry() to do it.
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] 6+ messages in thread
* [PATCH 2/4] libnvdimm: eliminate a meaningless spinlock operation
2020-08-20 14:30 [PATCH 0/4] clean up some functions in nvdimm/badrange.c and namespace_devs.c Zhen Lei
2020-08-20 14:30 ` [PATCH 1/4] libnvdimm: remove redundant list_empty() check in badrange.c Zhen Lei
@ 2020-08-20 14:30 ` Zhen Lei
2020-08-20 14:30 ` [PATCH 3/4] libnvdimm: eliminate two unnecessary zero initializations in badrange.c Zhen Lei
2020-08-20 14:30 ` [PATCH 4/4] libnvdimm: avoid unnecessary judgments in nvdimm_namespace_disk_name() Zhen Lei
3 siblings, 0 replies; 6+ messages in thread
From: Zhen Lei @ 2020-08-20 14:30 UTC (permalink / raw)
To: Oliver O'Halloran, Dan Williams, Vishal Verma, Dave Jiang,
Ira Weiny, linux-nvdimm, linux-kernel
Cc: Zhen Lei
badrange_add() take the lock "badrange->lock", but it's released
immediately in add_badrange(), protect nothing. Because the static
function add_badrange() is only called by badrange_add(), so spread its
content into badrange_add(), and move "kfree(bre_new)" out of the lock
protection.
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] 6+ messages in thread
* [PATCH 3/4] libnvdimm: eliminate two unnecessary zero initializations in badrange.c
2020-08-20 14:30 [PATCH 0/4] clean up some functions in nvdimm/badrange.c and namespace_devs.c Zhen Lei
2020-08-20 14:30 ` [PATCH 1/4] libnvdimm: remove redundant list_empty() check in badrange.c Zhen Lei
2020-08-20 14:30 ` [PATCH 2/4] libnvdimm: eliminate a meaningless spinlock operation Zhen Lei
@ 2020-08-20 14:30 ` Zhen Lei
2020-08-27 6:41 ` Leizhen (ThunderTown)
2020-08-20 14:30 ` [PATCH 4/4] libnvdimm: avoid unnecessary judgments in nvdimm_namespace_disk_name() Zhen Lei
3 siblings, 1 reply; 6+ messages in thread
From: Zhen Lei @ 2020-08-20 14:30 UTC (permalink / raw)
To: Oliver O'Halloran, Dan Williams, Vishal Verma, Dave Jiang,
Ira Weiny, linux-nvdimm, linux-kernel
Cc: Zhen Lei
Currently, the "struct badrange_entry" has three members: start, length,
list. In append_badrange_entry(), "start" and "length" will be assigned
later, and "list" does not need to be initialized before calling
list_add_tail(). That means, the kzalloc() in badrange_add() or
alloc_and_append_badrange_entry() can be replaced with kmalloc(), because
the zero initialization is not required.
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
drivers/nvdimm/badrange.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/nvdimm/badrange.c b/drivers/nvdimm/badrange.c
index 7f78b659057902d..13145001c52ff39 100644
--- a/drivers/nvdimm/badrange.c
+++ b/drivers/nvdimm/badrange.c
@@ -37,7 +37,7 @@ static int alloc_and_append_badrange_entry(struct badrange *badrange,
{
struct badrange_entry *bre;
- bre = kzalloc(sizeof(*bre), flags);
+ bre = kmalloc(sizeof(*bre), flags);
if (!bre)
return -ENOMEM;
@@ -49,7 +49,7 @@ int badrange_add(struct badrange *badrange, u64 addr, u64 length)
{
struct badrange_entry *bre, *bre_new;
- bre_new = kzalloc(sizeof(*bre_new), GFP_KERNEL);
+ bre_new = kmalloc(sizeof(*bre_new), GFP_KERNEL);
spin_lock(&badrange->lock);
--
1.8.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4/4] libnvdimm: avoid unnecessary judgments in nvdimm_namespace_disk_name()
2020-08-20 14:30 [PATCH 0/4] clean up some functions in nvdimm/badrange.c and namespace_devs.c Zhen Lei
` (2 preceding siblings ...)
2020-08-20 14:30 ` [PATCH 3/4] libnvdimm: eliminate two unnecessary zero initializations in badrange.c Zhen Lei
@ 2020-08-20 14:30 ` Zhen Lei
3 siblings, 0 replies; 6+ messages in thread
From: Zhen Lei @ 2020-08-20 14:30 UTC (permalink / raw)
To: Oliver O'Halloran, Dan Williams, Vishal Verma, Dave Jiang,
Ira Weiny, linux-nvdimm, linux-kernel
Cc: Zhen Lei
suffix ? suffix : "" appears three times, it's easy to get rid of it
by initialize the local variable "suffix" to empty string.
To avoid having rows that exceed 80 columns, add a new local variable
"region_id".
No functional change, but it can reduce the code size.
Before:
text data bss dec hex filename
41749 3697 16 45462 b196 drivers/nvdimm/namespace_devs.o
After:
text data bss dec hex filename
41653 3697 16 45366 b136 drivers/nvdimm/namespace_devs.o
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
drivers/nvdimm/namespace_devs.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
index 6da67f4d641a27c..ef2800c5da4c99c 100644
--- a/drivers/nvdimm/namespace_devs.c
+++ b/drivers/nvdimm/namespace_devs.c
@@ -157,7 +157,8 @@ const char *nvdimm_namespace_disk_name(struct nd_namespace_common *ndns,
char *name)
{
struct nd_region *nd_region = to_nd_region(ndns->dev.parent);
- const char *suffix = NULL;
+ const char *suffix = "";
+ int region_id = nd_region->id;
if (ndns->claim && is_nd_btt(ndns->claim))
suffix = "s";
@@ -173,17 +174,14 @@ const char *nvdimm_namespace_disk_name(struct nd_namespace_common *ndns,
}
if (nsidx)
- sprintf(name, "pmem%d.%d%s", nd_region->id, nsidx,
- suffix ? suffix : "");
+ sprintf(name, "pmem%d.%d%s", region_id, nsidx, suffix);
else
- sprintf(name, "pmem%d%s", nd_region->id,
- suffix ? suffix : "");
+ sprintf(name, "pmem%d%s", region_id, suffix);
} else if (is_namespace_blk(&ndns->dev)) {
struct nd_namespace_blk *nsblk;
nsblk = to_nd_namespace_blk(&ndns->dev);
- sprintf(name, "ndblk%d.%d%s", nd_region->id, nsblk->id,
- suffix ? suffix : "");
+ sprintf(name, "ndblk%d.%d%s", region_id, nsblk->id, suffix);
} else {
return NULL;
}
--
1.8.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 3/4] libnvdimm: eliminate two unnecessary zero initializations in badrange.c
2020-08-20 14:30 ` [PATCH 3/4] libnvdimm: eliminate two unnecessary zero initializations in badrange.c Zhen Lei
@ 2020-08-27 6:41 ` Leizhen (ThunderTown)
0 siblings, 0 replies; 6+ messages in thread
From: Leizhen (ThunderTown) @ 2020-08-27 6:41 UTC (permalink / raw)
To: Oliver O'Halloran, Dan Williams, Vishal Verma, Dave Jiang,
Ira Weiny, linux-nvdimm, linux-kernel
I will drop this patch, because badrange_add() is unlikely to be called.
There's no need to care about trivial performance improvements.
On 2020/8/20 22:30, Zhen Lei wrote:
> Currently, the "struct badrange_entry" has three members: start, length,
> list. In append_badrange_entry(), "start" and "length" will be assigned
> later, and "list" does not need to be initialized before calling
> list_add_tail(). That means, the kzalloc() in badrange_add() or
> alloc_and_append_badrange_entry() can be replaced with kmalloc(), because
> the zero initialization is not required.
>
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
> drivers/nvdimm/badrange.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvdimm/badrange.c b/drivers/nvdimm/badrange.c
> index 7f78b659057902d..13145001c52ff39 100644
> --- a/drivers/nvdimm/badrange.c
> +++ b/drivers/nvdimm/badrange.c
> @@ -37,7 +37,7 @@ static int alloc_and_append_badrange_entry(struct badrange *badrange,
> {
> struct badrange_entry *bre;
>
> - bre = kzalloc(sizeof(*bre), flags);
> + bre = kmalloc(sizeof(*bre), flags);
> if (!bre)
> return -ENOMEM;
>
> @@ -49,7 +49,7 @@ int badrange_add(struct badrange *badrange, u64 addr, u64 length)
> {
> struct badrange_entry *bre, *bre_new;
>
> - bre_new = kzalloc(sizeof(*bre_new), GFP_KERNEL);
> + bre_new = kmalloc(sizeof(*bre_new), GFP_KERNEL);
>
> spin_lock(&badrange->lock);
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-08-27 6:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-20 14:30 [PATCH 0/4] clean up some functions in nvdimm/badrange.c and namespace_devs.c Zhen Lei
2020-08-20 14:30 ` [PATCH 1/4] libnvdimm: remove redundant list_empty() check in badrange.c Zhen Lei
2020-08-20 14:30 ` [PATCH 2/4] libnvdimm: eliminate a meaningless spinlock operation Zhen Lei
2020-08-20 14:30 ` [PATCH 3/4] libnvdimm: eliminate two unnecessary zero initializations in badrange.c Zhen Lei
2020-08-27 6:41 ` Leizhen (ThunderTown)
2020-08-20 14:30 ` [PATCH 4/4] libnvdimm: avoid unnecessary judgments in nvdimm_namespace_disk_name() 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).