Subject: Re: [PATCHv2] perf bpf: Fix endianness problem when loading
 parameters in prologue
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: wangnan0@huawei.com, linux-perf-users@vger.kernel.org,
        linux-kernel@vger.kernel.org, brueckner@linux.vnet.ibm.com
References: <20170814114644.82526-1-tmricht@linux.vnet.ibm.com>
 <20170814163915.GB4114@kernel.org>
From: Thomas-Mich Richter <tmricht@linux.vnet.ibm.com>
Organization: IBM LTC
Date: Tue, 15 Aug 2017 08:42:08 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.2.0
MIME-Version: 1.0
In-Reply-To: <20170814163915.GB4114@kernel.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-IE
Content-Transfer-Encoding: 8bit
Message-Id: <54005fd5-de74-e34e-5ea7-86aab759a63d@linux.vnet.ibm.com>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2560
Lines: 70

On 08/14/2017 06:39 PM, Arnaldo Carvalho de Melo wrote:
> Em Mon, Aug 14, 2017 at 01:46:44PM +0200, Thomas Richter escreveu:
>> Perf BPF prologue generator unconditionally fetches 8 bytes for function
>> parameters, which causes problem on big endian machine. Thomas gives a
>> detail analysis for this problem:
>>
>>  http://lkml.kernel.org/r/968ebda5-abe4-8830-8d69-49f62529d151@linux.vnet.ibm.com
>>
>> This patch parses the type of each argument and converts data from
>> memory to expected type.
>>
>> Now the test runs successfully on 4.13.0-rc5:
>> [root@s8360046 perf]# ./perf test  bpf
>> 38: BPF filter                                 :
>> 38.1: Basic BPF filtering                      : Ok
>> 38.2: BPF pinning                              : Ok
>> 38.3: BPF prologue generation                  : Ok
>> 38.4: BPF relocation checker                   : Ok
>> [root@s8360046 perf]#
>>
>> Signed-off-by: Thomas Richter <tmricht@linux.vnet.ibm.com>
>> Signed-off-by: Wang Nan <wangnan0@huawei.com>
>> Acked-by: Wang Nan <wangnan0@huawei.com>
>> Tested-by: Wang Nan <wangnan0@huawei.com>
> 
> 
> That is strange, who is the author of the patch? Also I think Tested-by
> is enough, being an even stronger form of Acked-by?
> 
> But then you also have Signed-off-by: Wang in there...
> 
> From Documentation/process/submitting-patches.rst:
> 
> ---------
> 
> 12) When to use Acked-by: and Cc:
> ---------------------------------
> 
> The Signed-off-by: tag indicates that the signer was involved in the
> development of the patch, or that he/she was in the patch's delivery path.
> 
> If a person was not directly involved in the preparation or handling of a
> patch but wishes to signify and record their approval of it then they can
> ask to have an Acked-by: line added to the patch's changelog.
> 
> Acked-by: is often used by the maintainer of the affected code when that
> maintainer neither contributed to nor forwarded the patch.
> 
> ------------------
> 
> If Wang wrote the original patch and you made it better working together
> with him, probably having both of you in Signed-off-by lines should be
> enough?
> 
> - Arnaldo
> 

Ok, my fault then.
Wang wrote to patch in the first place, I just fixed one line.
Should I resend the patch and delete the Acked-by/Tested-by lines
in the commit message?

Thanks

-- 
Thomas Richter, Dept 3303, IBM LTC Boeblingen Germany
--
Vorsitzende des Aufsichtsrats: Martina Koederitz 
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294