Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <f0f31c00-c4a2-1df2-01f7-4e74685ee019@fb.com>
Date:   Sat, 16 Jul 2022 18:46:34 -0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.11.0
Subject: Re: [PATCH bpf-next 5/5] selftests/bpf: Remove the casting about
 jited_ksyms and jited_linfo
Content-Language: en-US
To:     Pu Lehui <pulehui@huawei.com>, bpf@vger.kernel.org,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Cc:     Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Quentin Monnet <quentin@isovalent.com>,
        Martin KaFai Lau <kafai@fb.com>,
        Song Liu <songliubraving@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>,
        Jean-Philippe Brucker <jean-philippe@linaro.org>
References: <20220716125108.1011206-1-pulehui@huawei.com>
 <20220716125108.1011206-6-pulehui@huawei.com>
From:   Yonghong Song <yhs@fb.com>
In-Reply-To: <20220716125108.1011206-6-pulehui@huawei.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RDYrNUpFZ0VqQlFQazZiT0ZZTTdjZURnanIrbGFyVUxPL3crNE1hTm1oYmtZ?=
 =?utf-8?B?N3hpazA1TjFGcjJSK0oyV3ZOcWdNbkZRaTdRQ3FEUlBiOHFRbVEwUGltbmk4?=
 =?utf-8?B?Sm9GM2dnR3lzNkRCNkdQMk8rTW5FVUhIU0xXOWp4SFN0K0pZTG1rR0JLa29r?=
 =?utf-8?B?MVZKYjRUQTR6Zk0xVDVpNGRVVDk1Smk5VDhONm95WDNXMUJQL1BnSjZ4TVlE?=
 =?utf-8?B?L2h5Z3N2RnNOZzFWTDVhellJM1RWcXlaQUk3eTdpRm9xVzhhQ3E2R2ZnZEU3?=
 =?utf-8?B?Z0hIcDU0cmZLeFVlM2hnS0hyclQ4Q2tnanlVcURXVmdpc0tDWWc4dmVmaSs4?=
 =?utf-8?B?VTR5bUpYV0x6Q3JLN0FzL0NEVVNtY1UzZjUzN0NveFFnWlV0bjFtL1VCSVdZ?=
 =?utf-8?B?QThiWll0b21Fc3FORUorQU03YzFXdWV3QlFMbkg4SDNLQ0dSTTg4MnNmUFVx?=
 =?utf-8?B?NW8zMHBvWDE2NmlJS2wyT01mQW0rQ3B4Q3N2OTJnLzRhRm5lRVowOVpMcWxI?=
 =?utf-8?B?Njd2UEpRVUxpbEl5U1MxN3ZHdU9ieHgxSGV5MSs0WkJMdXVwckY0UlFjdjVh?=
 =?utf-8?B?SGVLU3FMN05meGhOZG56YWNIaFRZbk52a1hPR0dobUpJRnRyZG1iSENENE5o?=
 =?utf-8?B?cEF0enJYR2ZNOSs1T0lsaFNrREJ5dmZuNUpnVEdTWGwvUE82NnM1Vit4S2lD?=
 =?utf-8?B?aWp1aU9peU9MbFYzSmpid3JrNGdZMUpGUytWR1dOOWRaZXorczF0Z3ZiRmI4?=
 =?utf-8?B?WmZXV3JTKzRjbm5jVmhMTko4T29ocmFtaCtUdFFINFJjYk9GdkV3ZExNQ0Jq?=
 =?utf-8?B?NFRlUDIzWUl0S1RXVm8zOWFRdk9iSUtyZG55dGlvZi9qcVNjdzlxUldFdGNS?=
 =?utf-8?B?TUF3MzlPdFVIaE9QelZwNjR4RTJPZkJHT3Y0WjhJTFEzb1E5ZGFoZ0cyMWYr?=
 =?utf-8?B?YVhLa3ZlMUdSZ3ZVNksyN3VLd1Z6R3FFMzNWZkk1M1RHalhDdE50RGtDOThn?=
 =?utf-8?B?dXJSZFBhTDFSYkJhY2pyR3hSd1FuRVBXYnV0K2ovL2pIMTk3cU1lS1p0WTdC?=
 =?utf-8?B?SXdPU1JQeEJNVG5Ra0U4YmdZZHNLdVYxMEZ5cjI0VmNDeWZtSkFjYk8wNURY?=
 =?utf-8?B?a0xWbHJMUkpBU1RQdHVZR0hBV1g3c3N6MGVtZVNJQ0FOYXV1R2d4Wjh3WXFr?=
 =?utf-8?B?QS9JdnhveEhmVGNXeUt2U1NyaWFGUTc2SmhFRy90ZUFvNDVTYlVFSmdKdEI2?=
 =?utf-8?B?aE4yMm4vV1RDYVhBelA3dzdWeVc3SUd0SnF4SVlwNW1iQjZxK0lKcmxqSWFT?=
 =?utf-8?B?dWFYeU1jUXlleE5xL0U2bk8rN1pKUy9mNGxBZ3NiZ3M0RHQwazFMb1h3VkEx?=
 =?utf-8?B?NjlqNUdqVzJrWWpJR3hIMzhXZlkxOEZuNTVNTHhYcHliY0xJRDVORWJqZ0xH?=
 =?utf-8?B?SmZPdUNxZjFzY0kxSVBubk00b21qT1VmTnJnTEpqRFd2S2dpMkJ0VmtDT2Vm?=
 =?utf-8?B?M3pBWU51NnpXeGpxVU9STVI4MUpwVGovS25wZnFSUzdNa3hVYkEvbzZlMUQv?=
 =?utf-8?B?ZVpMUnRWTTJDRkYrR3NKY21iRWxrYnBVeDBreTduSHpBT2RRL0FhN09zWHRx?=
 =?utf-8?B?ajJhRTBNK3FsZWQvbzk0dk9zcnEzc0w5VUFuVHMzMVI4dml4ZzFsZ2o5Ui9E?=
 =?utf-8?B?R2hGdDM2cXNlMG9vV0NjQzloUDlNOFJkYTczMmlEMUhDVVZvT2xDdlNTTGwx?=
 =?utf-8?B?azJOcVpKNGhWK1UxUllEWnMwaUlPajdYTU95WFRoa1pFaFdrcG5McGtvTWEw?=
 =?utf-8?B?ampYTVBwc09HNlV0YXhEa3Ixc2NjK0FCcllEdXdnUVZNb0pXc3JtR1lQNHNX?=
 =?utf-8?B?RUZpVDJoMlNLRXNyZngxTVFJS2xicWxRSHV0MXZzb0NNQWU0YUNPaTc1em5r?=
 =?utf-8?B?TzMvUXQ3K3MwU0JoaUw3V2h1WDF1YnFNak5ET1lsdFlCcFZZZzQ3UU1idmZj?=
 =?utf-8?B?S2pPdlJ4ajZsVmJIdmk2L1FnL3oyMnFpTllqQkFkTEU2WEVLWDdoM2cwcEtk?=
 =?utf-8?B?TTFZRDBYRDdvTWRDelRiVEozRnB0Z2JrWkNscEF2ZXduaTZielAvdWh4aU14?=
 =?utf-8?Q?krZoNoAcK3EGCuMUwGGNaB4EU?=
X-OriginatorOrg: fb.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 05b122b9-0f08-4d8a-10dd-08da67962ff0
X-MS-Exchange-CrossTenant-AuthSource: SN6PR1501MB2064.namprd15.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Jul 2022 01:46:37.4613
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: lPb+4mph4tNfk7jwP6K0lOfKhWcGDDXpvHhK8LvrBdmqX2O3W0jqNmt32BDc2+X+
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR15MB2639
X-Proofpoint-GUID: GeJeDQCudw0oA1oYfwLiv_t6hJoIXRiM
X-Proofpoint-ORIG-GUID: GeJeDQCudw0oA1oYfwLiv_t6hJoIXRiM
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.883,Hydra:6.0.517,FMLib:17.11.122.1
 definitions=2022-07-16_21,2022-07-15_01,2022-06-22_01
X-Spam-Status: No, score=-2.7 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,
        SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 7/16/22 5:51 AM, Pu Lehui wrote:
> We have unified data extension operation of jited_ksyms and jited_linfo
> into zero extension, so there's no need to cast u64 memory address to
> long data type.

For subject, we are not 'Remove the casting ...'. What the code did is
to change the casting.

Also, I don't understand the above commit message. What does this mean
about 'data extension operation of jited_ksyms and jited_linfo into zero 
extension'?

In prog_tests/btf.c, we have a few other places to cast 
jited_linfo[...]/jited_ksyms[...] to 'long' type. Maybe casting
to 'unsigned long' is a better choice. Casting to 'unsigned long long'
of course will work, but is it necessary? Or you are talking about
64bit kernel and 32bit user space?

> 
> Signed-off-by: Pu Lehui <pulehui@huawei.com>
> ---
>   tools/testing/selftests/bpf/prog_tests/btf.c | 16 +++++++++-------
>   1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
> index e852a9df779d..db10fa1745d1 100644
> --- a/tools/testing/selftests/bpf/prog_tests/btf.c
> +++ b/tools/testing/selftests/bpf/prog_tests/btf.c
> @@ -6613,8 +6613,9 @@ static int test_get_linfo(const struct prog_info_raw_test *test,
>   	}
>   
>   	if (CHECK(jited_linfo[0] != jited_ksyms[0],
> -		  "jited_linfo[0]:%lx != jited_ksyms[0]:%lx",
> -		  (long)(jited_linfo[0]), (long)(jited_ksyms[0]))) {
> +		  "jited_linfo[0]:%llx != jited_ksyms[0]:%llx",
> +		  (unsigned long long)(jited_linfo[0]),
> +		  (unsigned long long)(jited_ksyms[0]))) {
>   		err = -1;
>   		goto done;
>   	}
> @@ -6632,16 +6633,17 @@ static int test_get_linfo(const struct prog_info_raw_test *test,
>   		}
>   
>   		if (CHECK(jited_linfo[i] <= jited_linfo[i - 1],
> -			  "jited_linfo[%u]:%lx <= jited_linfo[%u]:%lx",
> -			  i, (long)jited_linfo[i],
> -			  i - 1, (long)(jited_linfo[i - 1]))) {
> +			  "jited_linfo[%u]:%llx <= jited_linfo[%u]:%llx",
> +			  i, (unsigned long long)jited_linfo[i],
> +			  i - 1, (unsigned long long)(jited_linfo[i - 1]))) {
>   			err = -1;
>   			goto done;
>   		}
>   
>   		if (CHECK(jited_linfo[i] - cur_func_ksyms > cur_func_len,
> -			  "jited_linfo[%u]:%lx - %lx > %u",
> -			  i, (long)jited_linfo[i], (long)cur_func_ksyms,
> +			  "jited_linfo[%u]:%llx - %llx > %u",
> +			  i, (unsigned long long)jited_linfo[i],
> +			  (unsigned long long)cur_func_ksyms,
>   			  cur_func_len)) {
>   			err = -1;
>   			goto done;