Subject: Re: [PATCH] ethernet: mlx4: Delete an error message for a failed
 memory allocation in five functions
To: Julia Lawall <julia.lawall@lip6.fr>,
        Tariq Toukan <tariqt@mellanox.com>
Cc: SF Markus Elfring <elfring@users.sourceforge.net>,
        linux-rdma@vger.kernel.org, netdev@vger.kernel.org,
        LKML <linux-kernel@vger.kernel.org>,
        kernel-janitors@vger.kernel.org
References: <30191db0-4d99-0349-b66a-c7354ef90d50@users.sourceforge.net>
 <0fea8f2f-f8fc-ce2e-3d33-44227de3637a@mellanox.com>
 <alpine.DEB.2.20.1801030904290.5305@hadrien>
From: Tariq Toukan <tariqt@mellanox.com>
Message-ID: <adfc3229-d3f9-afb9-00e6-9f765af0cbe8@mellanox.com>
Date: Wed, 3 Jan 2018 13:24:59 +0200
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101
 Thunderbird/52.5.2
MIME-Version: 1.0
In-Reply-To: <alpine.DEB.2.20.1801030904290.5305@hadrien>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Jan 2018 11:25:04.9802
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 33abb6e4-6a62-4e00-a7b9-08d5529ca40b
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: a652971c-7d2e-4d9b-a6a4-d149256f461b
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB5PR05MB1141
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Return-Path: <linux-kernel-owner@vger.kernel.org>


On 03/01/2018 10:06 AM, Julia Lawall wrote:
> 
> 
> On Wed, 3 Jan 2018, Tariq Toukan wrote:
> 
>>
>>
>> On 01/01/2018 10:46 PM, SF Markus Elfring wrote:
>>> From: Markus Elfring <elfring@users.sourceforge.net>
>>> Date: Mon, 1 Jan 2018 21:42:27 +0100
>>>
>>> Omit an extra message for a memory allocation failure in these functions.
>>>
>>> This issue was detected by using the Coccinelle software.
>>>
>>> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
>>> ---
>>
>> Is this an issue? Why? What is your motivation?
>> These are error messages, very informative, appear only upon errors, and in
>> control flow.
> 
> Strings take up space.  Since there is a backtrace on an out of memory
> problem, if the string does not provide any more information than the
> position of the call, then there is not much added value.  I don't know
> what was the string in this case.  If it provides some additional
> information, then it would be reasonable to keep it.

I don't really accept this claim...

Short informative strings worth the tiny space they consume. It helps 
the users of our driver understand what went wrong in simple words, 
without the need to understand the role of the functions/callstack or 
being familiar with different parts of the driver code.

In addition, some out-of-memory errors are recoverable, even though 
their backtrace is also printed. For example, in function 
mlx4_en_create_cq (appears in patch) we have a first allocation attempt 
(kzalloc_node) and a fallback (kzalloc). I'd prefer to state a clear 
error message only when both have failed, because otherwise the user 
might be confused whether the backtrace should indicate a malfunctioning 
interface, or not.

Tariq

> 
> julia
> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>