On 23.02.21 22:14:35, Dejin Zheng wrote:

On Tue, Feb 23, 2021 at 09:02:54AM +0100, Robert Richter wrote:

quoted

On 22.02.21 23:14:15, Dejin Zheng wrote:

quoted

On Mon, Feb 22, 2021 at 11:56:08AM +0100, Robert Richter wrote:

quoted

On 20.02.21 00:46:49, Dejin Zheng wrote:

quoted

quoted

On 18.02.21 23:04:55, Dejin Zheng wrote:

quoted

quoted

quoted

+	if (!dr || !dr->enabled)

here checks whether the pci device is enabled.

What is the purpose of this? The device "is_managed" or not.

The device is managed or not by check whether "dr" is NULL. And
check the "dr->enabled" is for the PCI device enable. I think it
may not make sense to apply for irq vectors when PCI device is not
enabled.

I don't see how a disabled device affects in any way the release of
the irq vectors during device removal. dr is always non-null in case
the device is managed, a check isn't needed for that.

Yes, the disabled device does not affect release irq vectors, But
the disabled device affects apply for irq vectors, It is wrong to apply
for the irq vectors when the device is not enabled.

What is the scenario you have in mind here? What does happen then?
The typical use case is to pcim_enable_device() it and then add the
irq vectors. It is always enabled then.

Even if the device could wrongly be disabled, it does not affect the
device's release.

Also, how is this related to pcim? There isn't a check in
pci_alloc_irq_vectors() either for that case. 

Add this check can
facilitate developers to find problems as soon as possible.

No, there are many ways to shoot yourself in the foot. We cannot add
checks here and there for this, esp. at runtime. If there is a valid
reason that the device must always be enabled and we cannot assume
this is the case, then we could add a WARN_ON(). But I doubt that.

-Robert