* [PATCH] block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()
@ 2017-08-07 10:52 SF Markus Elfring
2017-08-07 15:10 ` Geoff Levand
0 siblings, 1 reply; 11+ messages in thread
From: SF Markus Elfring @ 2017-08-07 10:52 UTC (permalink / raw)
To: linuxppc-dev, Benjamin Herrenschmidt, Geoff Levand,
Michael Ellerman, Paul Mackerras
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 7 Aug 2017 12:37:01 +0200
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/block/ps3vram.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/block/ps3vram.c b/drivers/block/ps3vram.c
index e0e81cacd781..ba97d037279e 100644
--- a/drivers/block/ps3vram.c
+++ b/drivers/block/ps3vram.c
@@ -409,10 +409,8 @@ static int ps3vram_cache_init(struct ps3_system_bus_device *dev)
priv->cache.page_size = CACHE_PAGE_SIZE;
priv->cache.tags = kzalloc(sizeof(struct ps3vram_tag) *
CACHE_PAGE_COUNT, GFP_KERNEL);
- if (priv->cache.tags == NULL) {
- dev_err(&dev->core, "Could not allocate cache tags\n");
+ if (!priv->cache.tags)
return -ENOMEM;
- }
dev_info(&dev->core, "Created ram cache: %d entries, %d KiB each\n",
CACHE_PAGE_COUNT, CACHE_PAGE_SIZE / 1024);
--
2.13.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()
2017-08-07 10:52 [PATCH] block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init() SF Markus Elfring
@ 2017-08-07 15:10 ` Geoff Levand
2017-08-07 15:13 ` Joe Perches
2017-08-07 16:27 ` SF Markus Elfring
0 siblings, 2 replies; 11+ messages in thread
From: Geoff Levand @ 2017-08-07 15:10 UTC (permalink / raw)
To: SF Markus Elfring, linuxppc-dev, Benjamin Herrenschmidt,
Michael Ellerman, Paul Mackerras
Cc: LKML, kernel-janitors
On 08/07/2017 03:52 AM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 7 Aug 2017 12:37:01 +0200
>
> Omit an extra message for a memory allocation failure in this function.
>
> This issue was detected by using the Coccinelle software.
NACK
When a user asks me for help I would certainly like to get
'Could not allocate cache tags' as apposed to nothing, since
the return value of ps3vram_cache_init() is not checked.
If you want to make an improvement please add a check for
success of ps3vram_cache_init() in ps3vram_probe().
-Geoff
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()
2017-08-07 15:10 ` Geoff Levand
@ 2017-08-07 15:13 ` Joe Perches
2017-08-07 16:27 ` SF Markus Elfring
1 sibling, 0 replies; 11+ messages in thread
From: Joe Perches @ 2017-08-07 15:13 UTC (permalink / raw)
To: Geoff Levand, SF Markus Elfring, linuxppc-dev,
Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras
Cc: LKML, kernel-janitors
On Mon, 2017-08-07 at 08:10 -0700, Geoff Levand wrote:
> On 08/07/2017 03:52 AM, SF Markus Elfring wrote:
> > Omit an extra message for a memory allocation failure in this function.
> NACK
>
> When a user asks me for help I would certainly like to get
> 'Could not allocate cache tags' as apposed to nothing, since
> the return value of ps3vram_cache_init() is not checked.
You still get a dump_stack on alloc failure.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()
2017-08-07 15:10 ` Geoff Levand
2017-08-07 15:13 ` Joe Perches
@ 2017-08-07 16:27 ` SF Markus Elfring
2017-08-07 18:28 ` Geoff Levand
1 sibling, 1 reply; 11+ messages in thread
From: SF Markus Elfring @ 2017-08-07 16:27 UTC (permalink / raw)
To: Geoff Levand, linuxppc-dev
Cc: Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras, LKML,
kernel-janitors
>> Omit an extra message for a memory allocation failure in this function.
>>
>> This issue was detected by using the Coccinelle software.
>
> NACK
>
> When a user asks me for help I would certainly like to get
> 'Could not allocate cache tags' as apposed to nothing,
Do you find the default allocation failure report insufficient?
> since the return value of ps3vram_cache_init() is not checked.
Are there any more update candidates to consider for better exception handling?
Regards,
Markus
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()
2017-08-07 16:27 ` SF Markus Elfring
@ 2017-08-07 18:28 ` Geoff Levand
2017-08-07 18:34 ` SF Markus Elfring
0 siblings, 1 reply; 11+ messages in thread
From: Geoff Levand @ 2017-08-07 18:28 UTC (permalink / raw)
To: SF Markus Elfring, linuxppc-dev
Cc: Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras, LKML,
kernel-janitors
On 08/07/2017 09:27 AM, SF Markus Elfring wrote:
>>> Omit an extra message for a memory allocation failure in this function.
>>>
>>> This issue was detected by using the Coccinelle software.
>>
>> NACK
>>
>> When a user asks me for help I would certainly like to get
>> 'Could not allocate cache tags' as apposed to nothing,
>
> Do you find the default allocation failure report insufficient?
The default is OK. I didn't consider one would be triggered by
the kzalloc failure.
>> since the return value of ps3vram_cache_init() is not checked.
>
> Are there any more update candidates to consider for better exception handling?
The return of ps3vram_cache_init() should be checked. Feel
free to propose others.
-Geoff
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()
2017-08-07 18:28 ` Geoff Levand
@ 2017-08-07 18:34 ` SF Markus Elfring
2017-08-07 18:49 ` Geoff Levand
0 siblings, 1 reply; 11+ messages in thread
From: SF Markus Elfring @ 2017-08-07 18:34 UTC (permalink / raw)
To: Geoff Levand, linuxppc-dev
Cc: Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras, LKML,
kernel-janitors
>> Do you find the default allocation failure report insufficient?
>
> The default is OK.
Thanks for this information.
> I didn't consider one would be triggered by the kzalloc failure.
Do you reconsider any special system settings for further
software evolution then?
Regards,
Markus
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()
2017-08-07 18:34 ` SF Markus Elfring
@ 2017-08-07 18:49 ` Geoff Levand
2017-08-07 19:04 ` SF Markus Elfring
0 siblings, 1 reply; 11+ messages in thread
From: Geoff Levand @ 2017-08-07 18:49 UTC (permalink / raw)
To: SF Markus Elfring, linuxppc-dev
Cc: Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras, LKML,
kernel-janitors
On 08/07/2017 11:34 AM, SF Markus Elfring wrote:
>> I didn't consider one would be triggered by the kzalloc failure.
>
> Do you reconsider any special system settings for further
> software evolution then?
Sorry, I don't quite understand your question.
I think your original patch is OK, and I would appreciate if
you added a check for failure of ps3vram_cache_init() in
ps3vram_probe(). If you decide not to add that check I'll
create a patch for it later.
If this doesn't answer your question, could you please
rephrase it?
-Geoff
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()
2017-08-07 18:49 ` Geoff Levand
@ 2017-08-07 19:04 ` SF Markus Elfring
2017-08-07 20:17 ` Geoff Levand
2017-08-08 2:13 ` Michael Ellerman
0 siblings, 2 replies; 11+ messages in thread
From: SF Markus Elfring @ 2017-08-07 19:04 UTC (permalink / raw)
To: Geoff Levand, linuxppc-dev
Cc: Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras, LKML,
kernel-janitors
>>> I didn't consider one would be triggered by the kzalloc failure.
>>
>> Do you reconsider any special system settings for further
>> software evolution then?
>
> Sorry, I don't quite understand your question.
Do you try to configure the Linux error reporting to any special needs?
> I think your original patch is OK,
How does this feedback fit to the initial response “Not Applicable”?
https://patchwork.ozlabs.org/patch/798575/
> and I would appreciate if you added a check for failure of ps3vram_cache_init()
> in ps3vram_probe().
I unsure if this adjustment will need more software updates.
> If you decide not to add that check I'll create a patch for it later.
I am curious on who will pick this update candidate up as the next improvement.
Have you got any preferences for the exception handling there?
Regards,
Markus
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()
2017-08-07 19:04 ` SF Markus Elfring
@ 2017-08-07 20:17 ` Geoff Levand
2017-08-08 8:23 ` SF Markus Elfring
2017-08-08 2:13 ` Michael Ellerman
1 sibling, 1 reply; 11+ messages in thread
From: Geoff Levand @ 2017-08-07 20:17 UTC (permalink / raw)
To: SF Markus Elfring, linuxppc-dev
Cc: Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras, LKML,
kernel-janitors
On 08/07/2017 12:04 PM, SF Markus Elfring wrote:
>> I think your original patch is OK,
>
> How does this feedback fit to the initial response “Not Applicable”?
> https://patchwork.ozlabs.org/patch/798575/
I submitted your patch and a fix to ps3vram_probe() with
the other patches in my queue. Thanks for your contribution.
-Geoff
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()
2017-08-07 20:17 ` Geoff Levand
@ 2017-08-08 8:23 ` SF Markus Elfring
0 siblings, 0 replies; 11+ messages in thread
From: SF Markus Elfring @ 2017-08-08 8:23 UTC (permalink / raw)
To: Geoff Levand, linuxppc-dev
Cc: LKML, kernel-janitors, Benjamin Herrenschmidt, Jens Axboe,
Jim Paris, Michael Ellerman, Paul Mackerras
>> https://patchwork.ozlabs.org/patch/798575/
>
> I submitted your patch
Thanks for your constructive feedback.
https://patchwork.ozlabs.org/patch/798850/
> and a fix to ps3vram_probe() with the other patches in my queue.
I find it nice that you picked this change opportunity up after
a bit of discussion (before an other developer would eventually
have tackled it also).
“Check return of ps3vram_cache_init”
https://patchwork.ozlabs.org/patch/798853/
1. Unfortunately, I find that this specific update suggestion does not fit
to the Linux coding style convention.
“…
Do not unnecessarily use braces where a single statement will do.
…”
2. How do you think about to use the check “if (error)” instead?
3. Will an additional commit description be useful?
Regards,
Markus
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()
2017-08-07 19:04 ` SF Markus Elfring
2017-08-07 20:17 ` Geoff Levand
@ 2017-08-08 2:13 ` Michael Ellerman
1 sibling, 0 replies; 11+ messages in thread
From: Michael Ellerman @ 2017-08-08 2:13 UTC (permalink / raw)
To: SF Markus Elfring, Geoff Levand, linuxppc-dev
Cc: Benjamin Herrenschmidt, Paul Mackerras, LKML, kernel-janitors
SF Markus Elfring <elfring@users.sourceforge.net> writes:
>>>> I didn't consider one would be triggered by the kzalloc failure.
>>>
>>> Do you reconsider any special system settings for further
>>> software evolution then?
>>=20
>> Sorry, I don't quite understand your question.
>
> Do you try to configure the Linux error reporting to any special needs?
>
>
>> I think your original patch is OK,
>
> How does this feedback fit to the initial response =E2=80=9CNot Applicabl=
e=E2=80=9D?
> https://patchwork.ozlabs.org/patch/798575/
That comes from me, and means "I can't apply this patch", because it's
not a powerpc patch.
Looking at the maintainers output though maybe that is meant to go via
the powerpc tree.
cheers
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-08-08 8:38 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-07 10:52 [PATCH] block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init() SF Markus Elfring
2017-08-07 15:10 ` Geoff Levand
2017-08-07 15:13 ` Joe Perches
2017-08-07 16:27 ` SF Markus Elfring
2017-08-07 18:28 ` Geoff Levand
2017-08-07 18:34 ` SF Markus Elfring
2017-08-07 18:49 ` Geoff Levand
2017-08-07 19:04 ` SF Markus Elfring
2017-08-07 20:17 ` Geoff Levand
2017-08-08 8:23 ` SF Markus Elfring
2017-08-08 2:13 ` Michael Ellerman
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).