RE: [PATCH 2/5 V2] PCI: pciehp: check and wait port status out of DPC before handling DLLSC and PDC

[Zhao, Haifeng]

Sinan,
   I explained the reason why locks don't protect this case in the patch description part. 
Write side and read side hold different semaphore and mutex.

Thanks,
Ethan

-----Original Message-----
From: Sinan Kaya <okaya@xxxxxxxxxx> 
Sent: Sunday, September 27, 2020 11:28 PM
To: Zhao, Haifeng <haifeng.zhao@xxxxxxxxx>; bhelgaas@xxxxxxxxxx; oohall@xxxxxxxxx; ruscur@xxxxxxxxxx; lukas@xxxxxxxxx; andriy.shevchenko@xxxxxxxxxxxxxxx; stuart.w.hayes@xxxxxxxxx; mr.nuke.me@xxxxxxxxx; mika.westerberg@xxxxxxxxxxxxxxx; Keith Busch <keith.busch@xxxxxxxxx>
Cc: linux-pci@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Jia, Pei P <pei.p.jia@xxxxxxxxx>; ashok.raj@xxxxxxxxxxxxxxx; Kuppuswamy, Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxx>
Subject: Re: [PATCH 2/5 V2] PCI: pciehp: check and wait port status out of DPC before handling DLLSC and PDC

On 9/26/2020 11:28 PM, Ethan Zhao wrote:
> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
> index 53433b37e181..6f271160f18d 100644
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -710,8 +710,10 @@ static irqreturn_t pciehp_ist(int irq, void *dev_id)
>  	down_read(&ctrl->reset_lock);
>  	if (events & DISABLE_SLOT)
>  		pciehp_handle_disable_request(ctrl);
> -	else if (events & (PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC))
> +	else if (events & (PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC)) {
> +		pci_wait_port_outdpc(pdev);
>  		pciehp_handle_presence_or_link_change(ctrl, events);
> +	}
>  	up_read(&ctrl->reset_lock);

This looks like a hack TBH.

Lukas, Keith;

What is your take on this?
Why is device lock not protecting this situation?

Is there a lock missing in hotplug driver?

Sinan