* [PATCH 0/2] fbdev: Adjustments for fb_alloc_cmap_gfp()
@ 2023-05-23 20:11 Markus Elfring
2023-05-23 20:15 ` [PATCH 1/2] fbdev: Move two variable assignments in fb_alloc_cmap_gfp() Markus Elfring
2023-05-23 20:18 ` [PATCH 2/2] fbdev: Convert a variable initialisation into a later assignment " Markus Elfring
0 siblings, 2 replies; 5+ messages in thread
From: Markus Elfring @ 2023-05-23 20:11 UTC (permalink / raw)
To: kernel-janitors, linux-fbdev, dri-devel, Daniel Vetter, Helge Deller
Cc: LKML, cocci
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 23 May 2023 22:04:33 +0200
A few update suggestions were taken into account
from static source code analysis.
Markus Elfring (2):
Move two variable assignments
Convert a variable initialisation into a later assignment
drivers/video/fbdev/core/fbcmap.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
--
2.40.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] fbdev: Move two variable assignments in fb_alloc_cmap_gfp()
2023-05-23 20:11 [PATCH 0/2] fbdev: Adjustments for fb_alloc_cmap_gfp() Markus Elfring
@ 2023-05-23 20:15 ` Markus Elfring
2023-05-24 18:10 ` Helge Deller
2023-05-23 20:18 ` [PATCH 2/2] fbdev: Convert a variable initialisation into a later assignment " Markus Elfring
1 sibling, 1 reply; 5+ messages in thread
From: Markus Elfring @ 2023-05-23 20:15 UTC (permalink / raw)
To: kernel-janitors, linux-fbdev, dri-devel, Daniel Vetter, Helge Deller
Cc: LKML, cocci
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 23 May 2023 21:30:29 +0200
Move the assignment for the local variables “size” and “flags”
because the computed values were only used in a single if branch.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/video/fbdev/core/fbcmap.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/video/fbdev/core/fbcmap.c b/drivers/video/fbdev/core/fbcmap.c
index ff09e57f3c38..5c1075ed28ab 100644
--- a/drivers/video/fbdev/core/fbcmap.c
+++ b/drivers/video/fbdev/core/fbcmap.c
@@ -91,16 +91,17 @@ static const struct fb_cmap default_16_colors = {
int fb_alloc_cmap_gfp(struct fb_cmap *cmap, int len, int transp, gfp_t flags)
{
- int size = len * sizeof(u16);
int ret = -ENOMEM;
- flags |= __GFP_NOWARN;
-
if (cmap->len != len) {
+ int size;
+
fb_dealloc_cmap(cmap);
if (!len)
return 0;
+ size = len * sizeof(u16);
+ flags |= __GFP_NOWARN;
cmap->red = kzalloc(size, flags);
if (!cmap->red)
goto fail;
--
2.40.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] fbdev: Convert a variable initialisation into a later assignment in fb_alloc_cmap_gfp()
2023-05-23 20:11 [PATCH 0/2] fbdev: Adjustments for fb_alloc_cmap_gfp() Markus Elfring
2023-05-23 20:15 ` [PATCH 1/2] fbdev: Move two variable assignments in fb_alloc_cmap_gfp() Markus Elfring
@ 2023-05-23 20:18 ` Markus Elfring
1 sibling, 0 replies; 5+ messages in thread
From: Markus Elfring @ 2023-05-23 20:18 UTC (permalink / raw)
To: kernel-janitors, linux-fbdev, dri-devel, Daniel Vetter, Helge Deller
Cc: LKML, cocci
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 23 May 2023 21:56:55 +0200
* Assign the value “-ENOMEM” to the local variable “ret” only for
the exception handling.
* Use an additional label.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/video/fbdev/core/fbcmap.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/video/fbdev/core/fbcmap.c b/drivers/video/fbdev/core/fbcmap.c
index 5c1075ed28ab..fbe82a64cf73 100644
--- a/drivers/video/fbdev/core/fbcmap.c
+++ b/drivers/video/fbdev/core/fbcmap.c
@@ -91,7 +91,7 @@ static const struct fb_cmap default_16_colors = {
int fb_alloc_cmap_gfp(struct fb_cmap *cmap, int len, int transp, gfp_t flags)
{
- int ret = -ENOMEM;
+ int ret;
if (cmap->len != len) {
int size;
@@ -104,17 +104,20 @@ int fb_alloc_cmap_gfp(struct fb_cmap *cmap, int len, int transp, gfp_t flags)
flags |= __GFP_NOWARN;
cmap->red = kzalloc(size, flags);
if (!cmap->red)
- goto fail;
+ goto e_nomem;
+
cmap->green = kzalloc(size, flags);
if (!cmap->green)
- goto fail;
+ goto e_nomem;
+
cmap->blue = kzalloc(size, flags);
if (!cmap->blue)
- goto fail;
+ goto e_nomem;
+
if (transp) {
cmap->transp = kzalloc(size, flags);
if (!cmap->transp)
- goto fail;
+ goto e_nomem;
} else {
cmap->transp = NULL;
}
@@ -126,6 +129,8 @@ int fb_alloc_cmap_gfp(struct fb_cmap *cmap, int len, int transp, gfp_t flags)
goto fail;
return 0;
+e_nomem:
+ ret = -ENOMEM;
fail:
fb_dealloc_cmap(cmap);
return ret;
--
2.40.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] fbdev: Move two variable assignments in fb_alloc_cmap_gfp()
2023-05-23 20:15 ` [PATCH 1/2] fbdev: Move two variable assignments in fb_alloc_cmap_gfp() Markus Elfring
@ 2023-05-24 18:10 ` Helge Deller
2023-05-25 5:02 ` Markus Elfring
0 siblings, 1 reply; 5+ messages in thread
From: Helge Deller @ 2023-05-24 18:10 UTC (permalink / raw)
To: Markus Elfring, kernel-janitors, linux-fbdev, dri-devel, Daniel Vetter
Cc: LKML, cocci
On 5/23/23 22:15, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 23 May 2023 21:30:29 +0200
>
> Move the assignment for the local variables “size” and “flags”
> because the computed values were only used in a single if branch.
Please do not move such variables without real need.
It makes backporting (of this and maybe follow-up patches) much more
complicated and the compiler will optimize it anyway.
Thanks!
Helge
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> drivers/video/fbdev/core/fbcmap.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/fbcmap.c b/drivers/video/fbdev/core/fbcmap.c
> index ff09e57f3c38..5c1075ed28ab 100644
> --- a/drivers/video/fbdev/core/fbcmap.c
> +++ b/drivers/video/fbdev/core/fbcmap.c
> @@ -91,16 +91,17 @@ static const struct fb_cmap default_16_colors = {
>
> int fb_alloc_cmap_gfp(struct fb_cmap *cmap, int len, int transp, gfp_t flags)
> {
> - int size = len * sizeof(u16);
> int ret = -ENOMEM;
>
> - flags |= __GFP_NOWARN;
> -
> if (cmap->len != len) {
> + int size;
> +
> fb_dealloc_cmap(cmap);
> if (!len)
> return 0;
>
> + size = len * sizeof(u16);
> + flags |= __GFP_NOWARN;
> cmap->red = kzalloc(size, flags);
> if (!cmap->red)
> goto fail;
> --
> 2.40.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] fbdev: Move two variable assignments in fb_alloc_cmap_gfp()
2023-05-24 18:10 ` Helge Deller
@ 2023-05-25 5:02 ` Markus Elfring
0 siblings, 0 replies; 5+ messages in thread
From: Markus Elfring @ 2023-05-25 5:02 UTC (permalink / raw)
To: Helge Deller, kernel-janitors, linux-fbdev, dri-devel, Daniel Vetter
Cc: LKML, cocci
>> Move the assignment for the local variables “size” and “flags”
>> because the computed values were only used in a single if branch.
>
> Please do not move such variables without real need.
Is there a need to explain desirable effects better?
> It makes backporting (of this and maybe follow-up patches) much more complicated
I suggest to reconsider such development concerns a bit more.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/drivers/video/fbdev/core/fbcmap.c?h=v6.4-rc3
> and the compiler will optimize it anyway.
How much do expectations fit to supported and documented software optimisations?
Regards,
Markus
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-05-25 5:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-23 20:11 [PATCH 0/2] fbdev: Adjustments for fb_alloc_cmap_gfp() Markus Elfring
2023-05-23 20:15 ` [PATCH 1/2] fbdev: Move two variable assignments in fb_alloc_cmap_gfp() Markus Elfring
2023-05-24 18:10 ` Helge Deller
2023-05-25 5:02 ` Markus Elfring
2023-05-23 20:18 ` [PATCH 2/2] fbdev: Convert a variable initialisation into a later assignment " Markus Elfring
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).