Re: [PATCH v6 3/6] PCI: Consolidate delimiter handling into pci_dev_str_match()

["Krzysztof Wilczyński" <kwilczynski@kernel.org>]

Hello,

> +not_found:
> +	if (ret < 0)
> +		pr_err("PCI: Can't parse parameter: %s\n", p);

Two issues here:

  - At this time, when this messages is to be shown, the p can be already
    at the NULL-terminator, as a result of parsing, so if the entire
    parameter is some garbage value, then it will show nothing in dmesg.

  - This new per_err() is not rate-limited, like the old messages were,
    so if the parameter is invalid or malformed, and you have... 30, or
    more on servers and serious hardware, devices in the system, then
    it will print for every single one of them.

Albeit, the second issue might not be an issue, and it might have been on
purpose.  However, I am not convinced it was.

> +	if (*p != ';' && *p != ',') {
> +		/*
> +		 * End of param or invalid format. Return -ENODEV so the caller
> +		 * stops parsing.
> +		 */
> +		return -ENODEV;
> +	}

[...]
> @@ -943,18 +960,11 @@ static void __pci_config_acs(struct pci_dev *dev, struct pci_acs *caps,
>  
>  		ret = pci_dev_str_match(dev, p, &p);
>  		if (ret < 0) {
> -			pr_info_once("PCI: Can't parse ACS command line parameter\n");


[...]
> @@ -6392,16 +6402,8 @@ static resource_size_t pci_specified_resource_alignment(struct pci_dev *dev,
>  			align = 1ULL << align_order;
>  			break;
>  		} else if (ret < 0) {
> -			pr_err("PCI: Can't parse resource_alignment parameter: %s\n",
> -			       p);

For the two above - it was nice to have the information about which
parameter to what kernel boot option was actually causing issues.  The
new code will only print a generic message.

Thank you!

	Krzysztof