Re: [PATCH v2] zfcp: fix reaction on bit error theshold notification with adapter close

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux