Re: [PATCH RESEND v2 5/7] PCI/sysfs: Allow userspace to query and set device reset mechanism

From: Krzysztof Wilczyński
Date: Thu May 20 2021 - 12:37:45 EST


Hi Amey,

[...]
> + if (sysfs_streq(buf, "")) {
> + pci_warn(pdev, "All device reset methods disabled by user");
> + goto set_reset_methods;
> + }

The sysfs_streq() is nice, indeed.

> + while ((name = strsep((char **)&buf, ",")) != NULL) {

I believe we could make this parsing a little bit more resilient,
especially since we are handling user input and this cannot ever be
really fully trusted, so for example:

while ((name = strsep((char **)&buf, ","))) {
if !(strlen(name)) <--- sysfs_streq() could be used here too.
continue;

name = strim(name); <--- remove leading and trailing whitespaces, if any.
(...)

[...]
> + if (reset_methods[0] &&
> + reset_methods[0] != PCI_RESET_FN_METHODS)
> + pci_warn(pdev, "Device specific reset disabled/de-prioritized by user");

What would be difference between disabling and de-prioritizing, is there
be a way for us to distinguish between the two? I was wondering if we
could, notify the user when the device specific reset is disable or when
it has been de-prioritized?

Krzysztof