Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1705348 > unrolled thread

[PATCH] block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()

Back to article view | Back to linux.kernel

Contents

  [PATCH] block/ps3vram: Delete an error message for a failed memory  allocation in ps3vram_cache_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-08-07 13:00 +0200
    Re: [PATCH] block/ps3vram: Delete an error message for a failed  memory allocation in ps3vram_cache_init() Joe Perches <joe@perches.com> - 2017-08-07 17:20 +0200
    Re: [PATCH] block/ps3vram: Delete an error message for a failed  memory allocation in ps3vram_cache_init() Geoff Levand <geoff@infradead.org> - 2017-08-07 17:20 +0200
      Re: block/ps3vram: Delete an error message for a failed memory  allocation in ps3vram_cache_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-08-07 18:30 +0200
        Re: block/ps3vram: Delete an error message for a failed memory  allocation in ps3vram_cache_init() Geoff Levand <geoff@infradead.org> - 2017-08-07 20:30 +0200
          Re: block/ps3vram: Delete an error message for a failed memory  allocation in ps3vram_cache_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-08-07 20:40 +0200
            Re: block/ps3vram: Delete an error message for a failed memory  allocation in ps3vram_cache_init() Geoff Levand <geoff@infradead.org> - 2017-08-07 20:50 +0200
              Re: block/ps3vram: Delete an error message for a failed memory  allocation in ps3vram_cache_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-08-07 21:10 +0200
                Re: block/ps3vram: Delete an error message for a failed memory  allocation in ps3vram_cache_init() Geoff Levand <geoff@infradead.org> - 2017-08-07 22:20 +0200
                  Re: block/ps3vram: Delete an error message for a failed memory  allocation in ps3vram_cache_init() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-08-08 10:30 +0200
                Re: block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init() Michael Ellerman <mpe@ellerman.id.au> - 2017-08-08 04:20 +0200

#1705348 — [PATCH] block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()

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

[toc] | [next] | [standalone]

#1705625 — Re: [PATCH] block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()

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.

[toc] | [prev] | [next] | [standalone]

#1705632 — Re: [PATCH] block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()

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

[toc] | [prev] | [next] | [standalone]

#1705693 — Re: block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()

>> 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

[toc] | [prev] | [next] | [standalone]

#1705777 — Re: block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()

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

[toc] | [prev] | [next] | [standalone]

#1705780 — Re: block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()

>> 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

[toc] | [prev] | [next] | [standalone]

#1705796 — Re: block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()

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

[toc] | [prev] | [next] | [standalone]

#1705811 — Re: block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()

>>> 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

[toc] | [prev] | [next] | [standalone]

#1705829 — Re: block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()

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

[toc] | [prev] | [next] | [standalone]

#1706150 — Re: block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()

>> 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

[toc] | [prev] | [next] | [standalone]

#1705965 — Re: block/ps3vram: Delete an error message for a failed memory allocation in ps3vram_cache_init()

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?
>> 
>> 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/

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web