Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Message-ID: <d93c4614-1bbc-3a30-305e-28ff75d7fde2@oracle.com>
Date:   Wed, 13 Dec 2023 00:13:37 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: [RFC 2/2] PCI: acpiphp: slowdown hotplug if hotplugging multiple
 devices at a time
Content-Language: en-US
To:     Igor Mammedov <imammedo@redhat.com>
Cc:     linux-acpi@vger.kernel.org, linux-pci@vger.kernel.org,
        mst@redhat.com, rafael@kernel.org, lenb@kernel.org,
        bhelgaas@google.com, mika.westerberg@linux.intel.com,
        boris.ostrovsky@oracle.com, joe.jin@oracle.com,
        stable@vger.kernel.org, Fiona Ebner <f.ebner@proxmox.com>,
        Thomas Lamprecht <t.lamprecht@proxmox.com>,
        linux-kernel@vger.kernel.org
References: <20231213003614.1648343-1-imammedo@redhat.com>
 <20231213003614.1648343-3-imammedo@redhat.com>
From:   Dongli Zhang <dongli.zhang@oracle.com>
In-Reply-To: <20231213003614.1648343-3-imammedo@redhat.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VmtYT05hT1VOVmxCNjB3R3FielZXVnFIN2prSjNhYklmWWxoZEhWREp0bDZS?=
 =?utf-8?B?d2F2Qk8ySzU1TzNZYUZ0bThaVHJYUzIySGpOWWRYSWJRUmFYMHhTbTdVeWQr?=
 =?utf-8?B?VFRWTVdBblIrUGpUNlBGN2h0N3A5WmhKSWhYK3JBQTZicmc1eFFORThHSFpl?=
 =?utf-8?B?T3pmTmxEa3gyY0NsVWNTbDNUZkFJcEN2eHFiTUE4dk1NOW1SNnFjaHc4WGYz?=
 =?utf-8?B?bmRTM1Y0eTZxV2RKTDVoSkVHUVZaakFmTFJSVExEN0FDb3EweUc5OXMrTllk?=
 =?utf-8?B?VGk1WUNucVh0ZWlham56azcyU0N3L0NLQjhjVzlldnl4NDBBKzVwbE80M29l?=
 =?utf-8?B?NDl1QmpYNitmck5ra2V6VkxlZTRRNEk4ZXg0TlBmNDVCOFVxMmxDTjRtcUJr?=
 =?utf-8?B?bGlDQjZvNjlGNkxvanpmeERkSWFndmlzSVEyYlpiV254Yk5Wc0xhNm54UEhW?=
 =?utf-8?B?b2F6YWh5WS93L1pheGpjVVd4ZklGUHI4UUxHbTVOZmhnSXhmdnhuQlFZSXNr?=
 =?utf-8?B?SjhLVlZoMHpqdUNSeWZQQndNRncwQWNJd053WmNKc0VNTTZvY2Rlb0ZqbXZt?=
 =?utf-8?B?TCsvV0lua1pVdElRNGt2ZVp1M09aek44TTZWK1lRQXN4N21FRDhiWXFQU3cz?=
 =?utf-8?B?UTgwODU1a09PNmRHbU95VUZ4RUtEZ0JNN0VyaDlSenQxL0JoQTd6MzI4OGJ4?=
 =?utf-8?B?dnAzblNLanUrTGZhdlVCQldjeCs3UDlqYy95QzNoUnhpRXc0S1doU0pFWUpR?=
 =?utf-8?B?dmRhN2M5K0RlTG1pM3pVTllpQkxwOU8weElGeEk5a01DQ2dZTUt2UVRTc0ZT?=
 =?utf-8?B?QXBmMXdiRkl3SER1WEhydDJMUS9jcWxYaG1jU2NmQXBXVTBhVFoyeStWdGQ4?=
 =?utf-8?B?RHAwRzVHMytadG5FdU8xTllTK05jcTBCNjNXTDcwTzZnS0lFSk5HZ1VGUHlL?=
 =?utf-8?B?Q1kzM1BEeThkbmQ1elJBUnF2UnBka3NnWUF0MVZ4aStFZU5jZldNWGQraHNU?=
 =?utf-8?B?TG1RcllTREoxU2tNVDA5ME1zc3FwTlEveTFHeDhTSlYxSVBMb2w3Rm9oQTJp?=
 =?utf-8?B?WUNaZ0pUbU9IRWJQcmR0aS95V2lwUlVWNFd4YWpDRklMWlBvc0E0dW5IV0Zm?=
 =?utf-8?B?cGl2Y3lTWUNVaDFHbzBjWE53dEVReFhYd1lJTlZCNFlVWkxWdld0bXdaZ3ls?=
 =?utf-8?B?elpCMHZKRG5WaE1uQS9kNzNnNlJKVGk2ZGlaUzBzMU8wQk1CdHFtL1B2d1da?=
 =?utf-8?B?KzRFaFNaQmZzemt6OGVkekJuVXBYWWVMN1FBS2Vnc1ZYd2gxQ2hTUDl5M0Ex?=
 =?utf-8?B?dW9MUFFkSlFvMjh3ZzBpTVkrSFBCdHF6Slp0L2w5TGo3M2pOZWlJZjFWTUJz?=
 =?utf-8?B?bjBQMm5mN1ZUeUNCU2cvd2k5UE12Y0VUTGlkSFI2ajNYVUpCMHEwbHpQYjR4?=
 =?utf-8?B?Rzl6UWZmbTdRM2lNRkptL3N5Z3lwR0NJZlArYnFtZExpNHJ3NWdGSm8zWmpv?=
 =?utf-8?B?V0laaGx5ZjJZNkJzd3hvQzZyRXZaLzJNNzVpWlduOVQ1dWs5V21BMXlrd3Jo?=
 =?utf-8?B?clFnZHZNWEgxR3lQY0VHRCtjSm81SnpBb3ZwR3FxT1c0UkxOY3UxdENpbHZM?=
 =?utf-8?B?dGNnVXB2OTAwR2pZV2xrRGw4OGtoSGZFMC9lcldRZHFKYUFzcHh0ZUFvRHJT?=
 =?utf-8?B?NndzTmFFUDZyZkduT3c2VTJyUzJNV0ZmU2NRSjQyQjVJL2lVNVFWc1hsbkQ1?=
 =?utf-8?B?NnFtZHIzcU50WFBKMUlVem95T0pDcU1hQjRQVGttUnZYRUVKaFFCMnh6RXlU?=
 =?utf-8?B?M1pLcFBCc2NRVUo5WG5UdzBiOXNNTG5CRFZwU1FxeTRzRFl4N0ZpbmxlNW1N?=
 =?utf-8?B?ZVFGelVvQkx4bEI0RytXcldBNG1uMkI3Ui9EZklEakQ4UGNjdlE2QytmMU1I?=
 =?utf-8?B?aFRJalVDOU1keUt0ZktBb0xyM05nQ1pXS2NuNkpUT0ZaQVUxdnNNc1JRd3hG?=
 =?utf-8?B?Smc1VnU2VXp2Z1ppNlVQbHV6REcrWGY5LzZDaEwwUjlpK2tVN0t5dUxUcE1M?=
 =?utf-8?B?Rk4yK0p1ZGFhTVRZY0hLK3dVNWd6a1M2c2dpakRRMGxkL3YybThQbWw0VUxi?=
 =?utf-8?Q?9sjq/BAVLZMNLB5dass9mFd+d?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: hANal5ykYDqUCV8p1jQ7jk3Ku7iS2eHTepdFODQTGtH4L2qfhqmVmtvTyAfWiSsgLE4d1s+ZClmm0Vy/hqtliAfIWgUeUTLKDJsNzcbFAmkcRP1Ay5fwgf2++6vtz/ceU31/FKN8JnJEA5avQmCdIDuRpXbg4Pu80SwHJAqbVCnmFNc+jYp0fu6PhsJPAwe6q/tVRbWTN5Ql1pkTPAjGps/6wmLkFpp8bngnA/g2Kzbzp4Dwty+6s0Sw6onbXfu0pEW8+0RoRvmw4MzF2yYCfyfr5Naj2VuYH9L7xzlyogr3ThNj0t1JL/PnrU2JamzvJewU2bapru0SRTa/VTo364OD8XvHuO7oGriZzmfRlzKgWwjg0X4kb53RgjRE9M27mtF8xa64IfIcaO+p0RE8X5SNMK+cL5OfpaT2sdQvC3YLh4W8qs9O4Xl9nnYWp9M3k98OZEX8n52/PrKdyEr+enI8drZsq4H+VWFzlJXM/ef6gb4l+opQ/s9qvOW/Blca3T7jso+Xh3fsnzZfox2Y53c11kHiLCk7QUR8oTs2sSOSIi6B8U94FNoh3U77MOob6U0xzm8fucyX9h0oXBxAQRxvHT1bgfWpk5TF+uKWU6o=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 77058b07-b26d-44f7-4397-08dbfbb36ae2
X-MS-Exchange-CrossTenant-AuthSource: BYAPR10MB2663.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Dec 2023 08:13:41.5159
 (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: nkM+GN9wm1MMmk9h4bnVyuqJhwzB/Lh0b2vGTgoVzmYLazdNTLRJS4fEe2d6jgRhS5wpr+rPkUYDdzPeU6sP9w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN0PR10MB5096
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-12-12_14,2023-12-12_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 mlxscore=0 suspectscore=0
 phishscore=0 adultscore=0 mlxlogscore=999 bulkscore=0 malwarescore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311290000
 definitions=main-2312130058
X-Proofpoint-GUID: OjxC00Pm3sxhyGZHzaVQm72sBZu9cVpq
X-Proofpoint-ORIG-GUID: OjxC00Pm3sxhyGZHzaVQm72sBZu9cVpq
X-Spam-Status: No, score=-3.4 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on pete.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (pete.vger.email [0.0.0.0]); Wed, 13 Dec 2023 00:14:27 -0800 (PST)

Hi Igor,


On 12/12/23 16:36, Igor Mammedov wrote:
> previous commit ("PCI: acpiphp: enable slot only if it hasn't been enabled already"
> introduced a workaround to avoid a race between SCSI_SCAN_ASYNC job and
> bridge reconfiguration in case of single HBA hotplug.
> However in virt environment it's possible to pause machine hotplug several
> HBAs and let machine run. That can hit the same race when 2nd hotplugged

Would you mind helping explain what does "pause machine hotplug several HBAs and
let machine run" indicate?

Thank you very much!

Dongli Zhang

> HBA will start re-configuring bridge.
> Do the same thing as SHPC and throttle down hotplug of 2nd and up
> devices within single hotplug event.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  drivers/pci/hotplug/acpiphp_glue.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
> index 6b11609927d6..30bca2086b24 100644
> --- a/drivers/pci/hotplug/acpiphp_glue.c
> +++ b/drivers/pci/hotplug/acpiphp_glue.c
> @@ -37,6 +37,7 @@
>  #include <linux/mutex.h>
>  #include <linux/slab.h>
>  #include <linux/acpi.h>
> +#include <linux/delay.h>
>  
>  #include "../pci.h"
>  #include "acpiphp.h"
> @@ -700,6 +701,7 @@ static void trim_stale_devices(struct pci_dev *dev)
>  static void acpiphp_check_bridge(struct acpiphp_bridge *bridge)
>  {
>  	struct acpiphp_slot *slot;
> +        int nr_hp_slots = 0;
>  
>  	/* Bail out if the bridge is going away. */
>  	if (bridge->is_going_away)
> @@ -723,6 +725,10 @@ static void acpiphp_check_bridge(struct acpiphp_bridge *bridge)
>  
>  			/* configure all functions */
>  			if (slot->flags != SLOT_ENABLED) {
> +				if (nr_hp_slots)
> +					msleep(1000);
> +
> +                                ++nr_hp_slots;
>  				enable_slot(slot, true);
>  			}
>  		} else {