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

2017-08-08 Thread SF Markus Elfring 
>> 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

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

2017-08-07 Thread Michael Ellerman 
SF Markus Elfring  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

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

2017-08-07 Thread Geoff Levand 
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

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

2017-08-07 Thread SF Markus Elfring 
>>> 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

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

2017-08-07 Thread Geoff Levand 
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

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

2017-08-07 Thread SF Markus Elfring 
>> 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

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

2017-08-07 Thread Geoff Levand 
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

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

2017-08-07 Thread SF Markus Elfring 
>> 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