Thank you for your contribution! Sashiko AI review found 1 potential issue to consider: - [Low] libmultipath: duplicate parser and ambiguous return in mpath_iopolicy_store() --
commit 12aacc0c8ac98ed4ae873a73979622db40d4f52b Author: John Garry <[email protected]> libmultipath: Add sysfs helpers This commit adds sysfs helpers and attribute group definitions for showing and updating the multipath I/O policy and displaying per-NUMA-node device paths. > diff --git a/lib/multipath.c b/lib/multipath.c > index cdf8fbc5105b3..ae5a73d570b33 100644 > --- a/lib/multipath.c > +++ b/lib/multipath.c > @@ -620,6 +620,100 @@ void mpath_device_set_live(struct mpath_device > *mpath_device) [ ... ] > +static void mpath_iopolicy_update(enum mpath_iopolicy_e *iopolicy, > + int new) > +{ > + int old = READ_ONCE(*iopolicy); > + > + if (old == new) > + return; > + > + WRITE_ONCE(*iopolicy, new); > + > + pr_info("iopolicy changed from %s to %s\n", > + mpath_iopolicy_names[old], > + mpath_iopolicy_names[new]); > +} > + > +bool mpath_iopolicy_store(enum mpath_iopolicy_e *iopolicy, const char *buf) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(mpath_iopolicy_names); i++) { > + if (sysfs_streq(buf, mpath_iopolicy_names[i])) { > + mpath_iopolicy_update(iopolicy, i); > + return true; > + } > + } > + > + return false; > +} > +EXPORT_SYMBOL_GPL(mpath_iopolicy_store); [Severity: Low] Can mpath_iopolicy_store() reuse mpath_iopolicy_parse(), and how can callers tell whether the policy actually changed? lib/multipath.c already defines mpath_iopolicy_parse() using __sysfs_match_string() to match against mpath_iopolicy_names[]. Also, because mpath_iopolicy_store() returns a bool that is true for any valid policy name even when old == new in mpath_iopolicy_update(), a sysfs store caller cannot both reject invalid strings with -EINVAL and avoid clearing cached paths and flushing requeue_work when the existing policy is written again. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=8
