Received-SPF: pass (google.com: domain of linux-kernel+bounces-78481-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <51483aaf-d64a-4eee-b256-ab126483ad6c@oracle.com>
Date: Fri, 23 Feb 2024 14:29:19 +0000
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] firmware_loader: Use init_utsname()->release
To: Luis Chamberlain <mcgrof@kernel.org>
Cc: russ.weight@linux.dev, gregkh@linuxfoundation.org, rafael@kernel.org,
        linux-kernel@vger.kernel.org, masahiroy@kernel.org
References: <20240222145819.96646-1-john.g.garry@oracle.com>
 <Zddt-U-6SdxkxqmD@bombadil.infradead.org>
Content-Language: en-US
From: John Garry <john.g.garry@oracle.com>
Organization: Oracle Corporation
In-Reply-To: <Zddt-U-6SdxkxqmD@bombadil.infradead.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?V1ErVStXK2ZYS09QbkZKT2dMaFptc3BNM0syNThBeGY3YzAyWE1YVGRHcFNm?=
 =?utf-8?B?T0NkNTdjbEZOVWpQUnY2TGkrY1pPaWNrdFh6SkhOeXhyRkRkcEp0aEJUMGRP?=
 =?utf-8?B?bmw3ZFFWTVdkV0ZGbU5KcjFNQzJiU1N1TExvYWQ2Ty90cU50UDkxVmFuN3li?=
 =?utf-8?B?REE4Y1psMjhNQ2ZwOGJGcVFZVDhGK2ltOGV1ZFY5bU5YNDlGREl4WmtuMks2?=
 =?utf-8?B?R09sS2hKdExXblU0VXBoWHNZZFdVQ2FXWG5EWGUzWVhacUNGWE4xbkE2aVM2?=
 =?utf-8?B?dnluYkpZMU5XOVJiSldkYm13eUxmclE3djBSYkhpOUVlcDRKVnJJbHRjb3dY?=
 =?utf-8?B?K1dWTzl0WnFHWXIrY3NDSCswRHhmaTdXcmNTV1N3c3NQdnVrS3pjdWxxQWtD?=
 =?utf-8?B?UVJaeWJMUUdDZXNrRHBRWG9TdjV1ZStMcHZNV2hHbU9PWml0b1B2SGoyeXkx?=
 =?utf-8?B?M2kweUoxVStLR1pNMnEyZmV2bzIvWHQzR0Q0S2ZuUUd5ek82MzYyUWcxaUE1?=
 =?utf-8?B?WnNhSzNyY0VaWDhXMVlWRnM5MjVTQWVGcU13YzlIZE9tZTNOMGRhei8zcFow?=
 =?utf-8?B?OGtDcXR5VzE0aUhzOVh5MXR6YTZ2cEZwT0c0R2hocHJyR1JRbUNHYlJnNUIy?=
 =?utf-8?B?b0J4T3BHQUpGd05lUzk1bGRHN2dxTDZiOW4xRXMxWWxJWExFZXpIQXJNTENR?=
 =?utf-8?B?YUFCNVlMWkZFYkRBZXBueGR1QWRRVEFPQ0RaZ2VrRjJ3TGNML3JqbW1lTHps?=
 =?utf-8?B?eGx6NndiOUVtTkRmWFdOcjZqZGdLT0pVZHMyVWtxenpDSDY4dk1mMkwyMXBG?=
 =?utf-8?B?eEpVeUw1RG5JREtzOW8vbmJNMFJxbnFkUFM1cnNIUkF4UXZzcVJCaytwWExh?=
 =?utf-8?B?NGVGbXFtMXd3T3VOdXk1ajh4MFZNbWxHSFdmbTlzMnZSVEZ6TExubVNuRGN6?=
 =?utf-8?B?ZVNpTkROZHhlUm1Ncm1nSTY2S0R4YVVIOVo0N3JXYkc4bXRsd0VQdk1KcUtK?=
 =?utf-8?B?QUd0aFZiWXUxQmdGbkZOeENBOFlKbUd5RVljSEtQNG9NVXBYSGZPZDgzR1VF?=
 =?utf-8?B?cjArUnBmeTVKcWIwZlYyZEJUSjlSaVhVZHZyaDhSVzlsTWF5Z0E4dUJFZmQ5?=
 =?utf-8?B?NEx2ZkxmZnZWSjdkckJwYkVJVC96dUxFZWxuTTFtekhzMTF1VUZzQXFMNEdM?=
 =?utf-8?B?TVRFamZXSWJudENSQW9pZ3RDOVdIK2hpcjZVUXNOUUVvNVlwRDdqbzA4dCsv?=
 =?utf-8?B?c292SnZFZ0NIbDRNV3dTanRONmxxR2hkYUIzNitPdzRueVo2cTBManFEYmZr?=
 =?utf-8?B?OUxselNIb2JrYmMyUHlXNTIwbytGZ2pFbUtrZ2xIT3hHSm15akIrQ1RqZ1ls?=
 =?utf-8?B?RmROM0pmTE5pcnhhQzlpVm1ORUR6eVNDRHNjby9nQUpub0tNT1dsUE8vRW9Q?=
 =?utf-8?B?MklHckg5NitHZll2SGQvalhNQW1qUkFWL0dPTUFFTTU0cHNyZDhtbklNZHlo?=
 =?utf-8?B?QWJ4dXhSeWtZUkhRcDQxM0lQcUJOckkzVEtCeTNjdmJQaThVMitia3ZzMG9o?=
 =?utf-8?B?UHV3OUlrMjZzUTc5b3F2QmNvTkxoZ0hiZjJFUFR6UkdjR2RVdldFVVI1QnB0?=
 =?utf-8?B?NVdOWWpVaHJYSlJlUGZwcWtnUWZsdy9UbVExV05vSWh0Wk9zSkg2eUt1ckN5?=
 =?utf-8?B?T21HbC9qdENsaDI2OFFyUmw0VEp2NXQ1bWl6cXdOR0cwc0VVMjBzSDZlOUJX?=
 =?utf-8?B?SnQxQ2ZySDJ6cmdMTGpjSmJjSkdqSEJMZ3AyRDJGc0FpalZUdUhyMFBiN2Ey?=
 =?utf-8?B?Zm94SVhvbXRqV1l0S0lzQnZqU1R4Z2NQNTlrUW5lNWliWVFNVUZWWnpmbUp1?=
 =?utf-8?B?TW5iSFlXbkVLQ0ZKR1NUclRyZElKSkRpNytvWVFOdVhyTWZ3eXdhcE9OYXFU?=
 =?utf-8?B?dnpHVFdxMDNMQVFidUgwUms1SmVnY2kxMGp3dkZMWUNGbExybm5HM0xzakc3?=
 =?utf-8?B?Um1yWDdZUzMxUGFKZGd2TFRSRXNMcW5EZmlWU3FKR2VuK1NXODc3M3N1Zm0w?=
 =?utf-8?B?c24yMFhJdUZmTHFKTjVlVDNSOFdHeFA5ZlVBU2JKdVpEZ3NCWFdDcDBLMjBw?=
 =?utf-8?Q?wpcL7F8iniOOhhobtUkHzxYSf?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	uc0r5Ch19E/v7UTttI7xxyek6KXTvy2/o575Jc2m0jTipVHgP4+/0KBNqIkLlbzDZ6G+VRggdQolpRFJl4HDLzYUtfL5igJVffOJCx5TmXVsOXpg6cIzj3948rqbRPuwfsLcYqN1N1zSdSPJUqvBIK248xYe+ErQwwo7+vETeHM+Hdw5yrDHK9x/lmf2ut0WnVhs/6IkNzpld7nEY43/zfAb++H50mWIuek5oor4hUbWSZf91nWEqUpKsyIHo/b/1NrHcG+8WhkgzJWYcx1hebwtTBQlBuYczbxVjlefo2MWfVOIQMxHcze98hwKhseA+6BKSsqR4p4GswJ7Fk8g9ideOGdMKJ8oTpKe2Ko5CeVvtWAW8KpD5FgJ+Oydj9wN/7UM4QmJu7a8O1pOUi5695WnNYXRDAjJqILLd/rHXJlreBlCY0j7oB7CI38aUjwsR/46JJLBNQ0/FlsKNNtB0JdXW5OM9kKWwtYeo4QBQQ5Nz/0m/qWce7HwaeiR3uOGuYgQaVCQvgp2+PnxvQhbng/YTLEuuVc+54Qm1yjLwPbwrEq/fNJ6ND2EaoyUiZYlzCV7v4oObB+8HiUGipx0FKleIcUiW4y399CWqiKsa9s=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9635f957-59ed-4b01-f33a-08dc347bd3db
X-MS-Exchange-CrossTenant-AuthSource: CH2PR10MB4312.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Feb 2024 14:29:22.2155
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 3Zk9yN+lj+GN1hbF6k/dhIlgJcCNHWG4UYKWhJf0+68zdzVoF7pTAIQSrSMuX6vr1Nej3+DltFi2jNB9gcstVA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR10MB6239
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-02-22_15,2024-02-23_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 suspectscore=0
 mlxscore=0 spamscore=0 mlxlogscore=999 phishscore=0 bulkscore=0
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2311290000 definitions=main-2402230106
X-Proofpoint-GUID: iZy8Qe-hk7CeVh6M11eFuPoFLEQCQ8zd
X-Proofpoint-ORIG-GUID: iZy8Qe-hk7CeVh6M11eFuPoFLEQCQ8zd


>> ---
>> Note: As mentioned by Masahiro in [0], when CONFIG_MODVERSIONS=y it
>> could be possible for a driver to be built as a module with a different
>> kernel baseline and so use a different UTS_RELEASE from the baseline. So
>> now using init_utsname()->release could lead to a change in behaviour
>> in this driver. However, considering the nature of this driver and how it
>> would not make sense to build as module against a different tree, this
> 
> would not make sense to build it as an external module against against a
> different tree, so this change should not have any effect to users.
> 

ok


>> change should be ok.
>>
>> [0] https://urldefense.com/v3/__https://lore.kernel.org/lkml/CAK7LNAQ_r5yUjNpOppLkDBQ12sDxBYQTvRZGn1ng8D1POfZr_A@mail.gmail.com/__;!!ACWV5N9M2RV99hQ!KxnFzsitgB8e0kDndt3nYwqw0FcAPKzJjuRl9BzwLQ9dbAtqK_SZOkhHw9ssT2PobYVkh8UU7WryWKwGXg$
> 
> Thanks for doing this, could you send a v2 with the "Note:" removed but
> instead include that blurb as part of the commit log as it is important
> information to retain.

ok

> 
> Could you also test the selftests to ensure nothing is broken ?
> 
> ./tools/testing/selftests/firmware/fw_run_tests.sh

I am running this now, but it does not seem stable on a v6.8-rc5 
baseline. I am seeing hangs. Are there any known issues?

This one passed, it seems to me:

https://pastebin.com/ZySPmH9h

But then on another run I see a hang at:

...
Testing with the file missing...
Batched request_firmware() nofile try #1: OK
Batched request_firmware() nofile try #2: OK
Batched request_firmware() nofile try #3: OK
Batched request_firmware() nofile try #4: OK
Batched request_firmware() nofile try #5: OK
Batched request_firmware_into_buf() nofile try #1: OK
Batched request_firmware_into_buf() nofile try #2: OK
Batched request_firmware_into_buf() nofile try #3: OK
Batched request_firmware_into_buf() nofile try #4: OK
Batched request_firmware_into_buf() nofile try #5: OK
Batched request_firmware_direct() nofile try #1: OK
Batched request_firmware_direct() nofile try #2: OK
Batched request_firmware_direct() nofile try #3: OK
Batched request_firmware_direct() nofile try #4: OK
Batched request_firmware_direct() nofile try #5: OK
Batched request_firmware_nowait(uevent=true) nofile try #1: OK
Batched request_firmware_nowait(uevent=true) nofile try #2: OK
Batched request_firmware_nowait(uevent=true) nofile try #3: OK
Batched request_firmware_nowait(uevent=true) nofile try #4: OK
Batched request_firmware_nowait(uevent=true) nofile try #5: OK
Batched request_firmware_nowait(uevent=false) nofile try #1: OK
Batched request_firmware_nowait(uevent=false) nofile try #2: OK
Batched request_firmware_nowait(uevent=false) nofile try #3: OK
Batched request_firmware_nowait(uevent=false) nofile try #4:


And kernel log:

[  385.105004] test_firmware: #3: failed to async load firmware
[  385.181221] test_firmware: #0: failed to async load firmware
[  385.252231] test_firmware: #1: failed to async load firmware
[  385.323307] test_firmware: #2: failed to async load firmware
[  385.397444] test_firmware: #3: failed to async load firmware
[  644.462125] INFO: task fw_filesystem.s:5466 blocked for more than 120 
seconds.
[  644.548803]       Not tainted 6.8.0-rc5-g7e58e46976f2 #32
[  644.613581] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" 
disables this message.
[  765.294106] INFO: task fw_filesystem.s:5466 blocked for more than 241 
seconds.
[  765.380805]       Not tainted 6.8.0-rc5-g7e58e46976f2 #32
[  765.445610] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" 
disables this message.
[  886.126154] INFO: task fw_filesystem.s:5466 blocked for more than 362 
seconds.
[  886.212883]       Not tainted 6.8.0-rc5-g7e58e46976f2 #32
[  886.277695] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" 
disables this message.

FWIW, with my patch we get same output for "passing" run upto another hang.

Thanks,
John

>