On Tue, Oct 01, 2019 at 05:07:50PM +0200, Steffen Maier wrote: > On 10/1/19 4:14 PM, Greg KH wrote: > > On Tue, Oct 01, 2019 at 12:49:49PM +0200, Steffen Maier wrote: > > > On excessive bit errors for the FCP channel ingress fibre path, the channel > > > notifies us. Previously, we only emitted a kernel message and a trace record. > > > Since performance can become suboptimal with I/O timeouts due to > > > bit errors, we now stop using an FCP device by default on channel > > > notification so multipath on top can timely failover to other paths. > > > A new module parameter zfcp.ber_stop can be used to get zfcp old behavior. > > > > Ugh, module parameters? This isn't the 1990's anymore :( > > > > Why not just make this a dynamic sysfs variable, that way you properly > > can set this on whatever device you want, not just "all or nothing"? > > Since we can see many more (virtual) FCP devices than we want to actually > use, we defer probing. It means, we only start allocating structures and > sysfs entries on setting an FCP "online" for the first time. Setting online > works through another sysfs attribute owned by our ccw bus code component > called "cio". IIRC, setting online does not emit a uevent. On setting > online, the (add) uevent of hot-/coldplug of an FCP device had already > happened, so we could not easily have end users craft udev rules to > automatically/persistently configure a new sysfs attribute (which is > FCP-device-specific and appears late) to disable the new code behavior. > > Not sure if that could ever become a problem for end users: Even if we were > to write into a new sysfs attribute, the attribute only appears during > setting online so this might race with starting to actually use the FCP > device with the new default behavior and could potentially disable I/O paths > before the sysfs attribute write could become effective to disable the new > behavor. Ok, then why make this a module option that you will have to support for the next 20+ years anyway if you feel this fix is the correct way that it should be done instead? module options are tough to manage and support, only add them as a very last thing, when all other options have been ruled out. thanks, greg k-h
