Received-SPF: pass (google.com: domain of linux-kernel+bounces-196602-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <1ecbda96-5b32-4c6f-9cd3-74f9b78cb9e2@intel.com>
Date: Fri, 31 May 2024 11:37:44 +0200
User-Agent: Mozilla Thunderbird
Subject: Re: [Intel-wired-lan] [PATCH] ice: irdma hardware init failed after
 suspend/resume
To: En-Wei WU <en-wei.wu@canonical.com>, Paul Menzel <pmenzel@molgen.mpg.de>
CC: <netdev@vger.kernel.org>, <rickywu0421@gmail.com>,
	<linux-kernel@vger.kernel.org>, <edumazet@google.com>,
	<anthony.l.nguyen@intel.com>, <kuba@kernel.org>,
	<intel-wired-lan@lists.osuosl.org>, <pabeni@redhat.com>,
	<davem@davemloft.net>
References: <20240528100315.24290-1-en-wei.wu@canonical.com>
 <88c6a5ee-1872-4c15-bef2-dcf3bc0b39fb@molgen.mpg.de>
 <CAMqyJG0uUgjN90BqjXSfgq7HD3ACdLwOM8P2B+wjiP1Zn1gjAQ@mail.gmail.com>
 <971a2c3b-1cd9-48c5-aa50-e3c441277f0a@molgen.mpg.de>
 <CAMqyJG13Q+20p5gPpLZ1JYBS6yt5HZox0=gaT87vDyxN1rxRyA@mail.gmail.com>
Content-Language: en-US
From: Wojciech Drewek <wojciech.drewek@intel.com>
In-Reply-To: <CAMqyJG13Q+20p5gPpLZ1JYBS6yt5HZox0=gaT87vDyxN1rxRyA@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZmVRZlZtNkhPbVBMR2dXMENFL0FOdVJkaVRwRUJMbWMzZDhaRXQxVStaVmo2?=
 =?utf-8?B?R3dndVRRZmxzOEQ2bEh0WDJTaXFkeWUrTnZxblgwakt6dTJzTjRkYktrVFU0?=
 =?utf-8?B?QkhJZmJnTFEzMElIRXNscjlBY3M0dG1JanBvdzJCaVB5Wlg5dDlyS1V5T3Zx?=
 =?utf-8?B?S1gxU2l6ZEx3bys1RWkwZHNUSWVDZ0czQlprdldjOWpmb1ZEazJVRG9KY1M5?=
 =?utf-8?B?Sk11bGJ3UmtjMHNhaG1hRnRycEZmSzVmWStvY29sVjAzV29NWDBPSHZJZis2?=
 =?utf-8?B?YldhYUhTS0cwVHdhWFFuUDZNYkFWU3BQck94UkhReDFxeXlUdHF4NFY4N3pH?=
 =?utf-8?B?Y1RIMFk0TlFnNFM3RGkwOGppRTVVWThCT2RSektjM1IyU3YzbWlSV1NIaGNy?=
 =?utf-8?B?eGZHODdnS2RROGNYbmRMTlp3dm9Bdy8rSG54N0JRQ1BCazNodmZqdXBlR1I0?=
 =?utf-8?B?d3JrN3hTUFZLYm1Ma25DVEdTelNvYlI1czlYdW1PcUVUNHNmQmpCU3hzdE11?=
 =?utf-8?B?WGZZNlNzVkpjMDEvYlIxMXJWKzh6dk53Rjk4Q3hCa2NpcUZsT2NRUGJEc1cz?=
 =?utf-8?B?S1VZeVowSTZhTTlUUkc1UXQxemdmMThadWhNcHlSUDMyV1luN0Z5bytKcUxa?=
 =?utf-8?B?MFN1TlBKZDF1a0hiUFFsTWZkT3U1K3p3K3RLdTlsTzFJQWV2TnI3RnFQRE4v?=
 =?utf-8?B?OFp5OStTaFZ3b2hncGVKMWFEZXBjVW96ODg5MGtlZG1JWTdVZlNFNzZvbEVq?=
 =?utf-8?B?b1RObTc5aWJQSVdHRTd4enhFTlA5ZjlsM2NxQjkxZmhNZmo4WjJHUzBXSEFV?=
 =?utf-8?B?ZUo3MjgxTCtoT2EvZ0lpM0R3czZLbkZrakVZYXdlem5NYnVRdUo3allBd3hz?=
 =?utf-8?B?S3ZsY1VGamd3RmZ0RElrUFJMUFQyQ1ljV2wvSEh1QnA1K3ArYlNvVERIRXM0?=
 =?utf-8?B?MUpzS2w4L1JWOE5xbnQvMjFKN0J3NUVYM3BlazVOS25Fa0dMWGxMRWxEV0Nm?=
 =?utf-8?B?Q2FIVVlwS09QcVNPOCthbmxMdWt4YnYzOWhCZjVOOHpLMGlGS0FlUzBXTTMw?=
 =?utf-8?B?OXBnZENmdnJnL1RqYnllZ1N4Ui9IQmJ2Y3dsSXQySFFpQVpZZWhvQ2FqbzRl?=
 =?utf-8?B?RTJJOGtOT3g0SExSYm9MVFVaY2VkcEdhTVh6NUpjMVpOYTJGaVJLNVhXS1p0?=
 =?utf-8?B?TXNwLzdEY3JsaXQyams4NHdoWFIxYVdvU1NxUjVCUlYxQ0l1bjFiTkJjZHUy?=
 =?utf-8?B?aHBSa0Rkekg3bkJHcnpuc3ZwT0lObzlNSUhzNVp5VWN4YUxsa2F0NDhnMCtK?=
 =?utf-8?B?UGxDMkdWc2FDT0JWb1oxb0JKWldYZTZnQjQxUitNUFlmUW5KTDZ0T1RmbGNj?=
 =?utf-8?B?cDVKWlpZbjhSVXZaemlYTlFwajNmTHJ4SDV3VGRMU3JwbHFNTzdJTExVY3po?=
 =?utf-8?B?OGZIUFNhK1FqelVtQWU4dnVZYi9nVGdrV1J0R2xvRGdmVkpnQmdLajJHUzBj?=
 =?utf-8?B?NjQrc1NueDAzMUFqbzhaNWpzRk11b1l3RVAzbVV1ak1ZS0dyd1lEMWpYeU9v?=
 =?utf-8?B?c2FIYjR3RHBDdzJVMnVsNDlMTXFCdFdOQlQ2bXBONWl1MW1oL1ErcUhsaDYx?=
 =?utf-8?B?S29MRWV4S3F0eVBOWnRETWxVS2pjdVFvck94akNkRy9CTHJPK0M4SVZDdUpt?=
 =?utf-8?B?TVJIT0s1NWRHMjFBcm55SnJRdlhZUEhTRm1wd2tTVHNnOERiSUR0TXlMNjJB?=
 =?utf-8?B?ckZ2ZGw0NjhWQ1dpTlVnYWdrd3d1YWMxKzNXY21YMDk2dVk5eHhwVTJ3SlUv?=
 =?utf-8?B?MFdjT3pPeVpCSkJhd2Y5WUdvWWVEV0FpNXhuaFg0emtneTZNNHUzSko1blN1?=
 =?utf-8?B?THV1aXVEQlhIcSszaTdISEk5N25PaW56Nis4ZHYwSTF6NUwwbUFuajVlVkhv?=
 =?utf-8?B?SENRbldsaVdidjZmbFNvMnkzaDNNMUNnSkM4VThKTXlmaGNRMmR1TmlPUnNx?=
 =?utf-8?B?OXQ0TTJpckRYdWhpbkk0aDYyZ20wOWp5R0psSERyOFFzc1djVkxYUDBxcFhy?=
 =?utf-8?B?QUJIOFNiODUreXJVcXRKNDdTZllRYjA0aExUSmkvdEdyeEVqZVBjOXEzYUJn?=
 =?utf-8?B?aTM2SkVOT3kwSWF0TjB4a09hV2I2eW5Mdm5JcUZHc09uL0dzb05lQ1JrQ2Zw?=
 =?utf-8?B?Znc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: e0a7e0fd-c6be-498d-94ef-08dc81555877
X-MS-Exchange-CrossTenant-AuthSource: MW4PR11MB5776.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 May 2024 09:37:53.8851
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: W+RKJcD5vB1uKLOS+9LA7a0r77cqOwJwTmm+pAuWdg5R52fECCllQ2suUaMd1p66Xrf5FkBQxn/0zqsR7r9hBZiIHZGgXsPRBKt2yxYhWfU=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB5969
X-OriginatorOrg: intel.com


On 30.05.2024 09:08, En-Wei WU wrote:
> Thank you for your reply.
> 
>> Sorry for being unclear. I meant, does resuming the system take longer
>> now? (initcall_debug might give a clue.)
> I've tested the S3 suspend/resume with the initcall_debug kernel
> command option, and it shows no clear difference between having or not
> having the ice_init_rdma in ice_resume:
> Without ice_init_rdma:
> ```
> [  104.241129] ice 0000:86:00.0: PM: pci_pm_resume+0x0/0x110 returned
> 0 after 9415 usecs
> [  104.241206] ice 0000:86:00.1: PM: pci_pm_resume+0x0/0x110 returned
> 0 after 9443 usecs
> ```
> With ice_init_rdma:
> ```
> [  122.749022] ice 0000:86:00.1: PM: pci_pm_resume+0x0/0x110 returned
> 0 after 9485 usecs
> [  122.749068] ice 0000:86:00.0: PM: pci_pm_resume+0x0/0x110 returned
> 0 after 9532 usecs
> ```
> 
>> And ice_init_rdma should be moved to ice_rebuild (replace ice_plug_aux_dev)
> We can defer the ice_init_rdma to the later service task by adopting this.
> 
>> You should call ice_deinit_rdma in ice_prepare_for_reset (replace ice_unplug_aux_dev),
> It seems like we must call ice_deinit_rdma in ice_suspend. If we call
> it in the later service task, it will:
> 1. break some existing code setup by ice_resume
> 2. Since the PCI-X vector table is flushed at the end of ice_suspend,
> we have no way to release PCI-X vectors for rdma if we had allocated
> it dynamically
> The second point is important since we didn't release the PCI-X
> vectors for rdma (if we allocated it dynamically) in the original
> ice_suspend, and it's somewhat like a leak in the original code.
> 
> Best regards,
> Ricky.

Makes sense, let's keep ice_deinit_rdma in ice_suspend.

> 
> On Thu, 30 May 2024 at 04:19, Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>>
>> Dear En-Wei,
>>
>>
>> Thank you for responding so quickly.
>>
>> Am 29.05.24 um 05:17 schrieb En-Wei WU:
>>
>> […]
>>
>>>> What effect does this have on resume time?
>>>
>>> When we call ice_init_rdma() at resume time, it will allocate entries
>>> at pf->irq_tracker.entries and update pf->msix_entries for later use
>>> (request_irq) by irdma.
>>
>> Sorry for being unclear. I meant, does resuming the system take longer
>> now? (initcall_debug might give a clue.)
>>
>>
>> Kind regards,
>>
>> Paul