Re: [PATCH] pci: msi: return error code instead of only warn

From: Christoph Hellwig <hch@lst.de>
To: xiangxia.m.yue@gmail.com
Cc: helgaas@kernel.org, hch@lst.de, linux-pci@vger.kernel.org
Subject: Re: [PATCH] pci: msi: return error code instead of only warn
Date: Tue, 25 Sep 2018 17:36:09 +0200	[thread overview]
Message-ID: <20180925153609.GB404@lst.de> (raw)
In-Reply-To: <1537797641-6868-1-git-send-email-xiangxia.m.yue@gmail.com>

On Mon, Sep 24, 2018 at 07:00:41AM -0700, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> 
> The drivers may enable the msi/msix more than once.
> Actually, this is the driver bug. But the bug may panic
> the kernel and should be fixed. We can return the error
> code instead of WARN_ON, to tell the drivers that the
> msi/msix is already enabled.
> 
> When enable them again, the call tree is shown on kernel
> 3.10.0, but linux upstream in the same way.

You can use up to 73 characters per line for your changelog, and doing so
makes it a little more readable.

> +	if (WARN_ON_ONCE(!!dev->msi_enabled))
> +		return  -EINVAL;
> +
>  	nvec = pci_msi_vec_count(dev);
>  	if (nvec < 0)
>  		return nvec;
> @@ -1087,6 +1087,9 @@ static int __pci_enable_msix_range(struct pci_dev *dev,
>  	if (maxvec < minvec)
>  		return -ERANGE;
>  
> +	if (WARN_ON_ONCE(!!dev->msix_enabled))
> +		return  -EINVAL;
> +

You can drop the "!!", WARN_ON_ONCE will do the implicit bool conversion
for you.

Except for that the patch looks good to me.