James Bottomley wrote:
On Mon, 2007-10-29 at 12:24 -0400, Jeff Garzik wrote:
James Bottomley wrote:
Ah, OK; I haven't communicated what we need very clearly. We need a way
to see if the event is supported by the device, as well as a way to turn
it off. For some of the events (possibly not the SATA AN one, since I
know all SATA devices will be well behaved) there's going to be a need
to deal with berserk or broken devices that become trigger happy, so
turning off the event will be a useful (and possibly essential) way of
coping.
That's possible with the presented interface[1]:
# see if event is supported
cat $path/evt_media_change
# turn off event to deal with broken/beserk devices
echo 0 > $path/evt_media_change
Some sillyhead can always do
echo 1 > $path/evt_some_event_my_device_does_not_support
but that will be obviously be a no-op because their device simply will
not send such events.
Granted ls(1) is no longer a method for viewing supported-at-boot-time
list of events -- ls(1) in the presented interface lists what events the
_kernel_ supports, and cat(1) is used to discover which events are
actually enabled.
I think that is the only difference between our two positions: [if I
understand you correctly] you want ls(1) to be able to list the device's
supported events. However, I feel that is inconsistent: for your
proposal, userspace must perform two checks in order to determine a
feature's availability: 1) does the file exist? 2) is the file context
non-zero?
Yes, I agree ... however, open file is one op for the user -ENXIO means
device doesn't support the event; value indicates whether the event is
currently triggering.
I just would rather we use the file exists if device supports event,
because it's consistent with all the rest of our SCSI interfaces.
Two problems with what you just described:
1) "value indicates current event state" is a new concept in this thread
(maybe you were thinking this all along, but I didn't get that from your
writing).
Watching the sysfs node for event activity is definitely outside the
scope of this work, and IMO not very useful. The time from when LLDD
calls sdev_evt_notify() until uevent completion is very short, so the
time window for actually receiving a useful value in your scenario is
also short.
My patch presented the attributes purely as control nodes, only affected
sdev->supported_events and nothing more. You seem to be suggesting
exporting the true-for-only-a-few-milliseconds activity state, rather
then enable/disable state.
2) Event support itself is dynamic, which causes me to revisit the
"complexity" argument. In libata, for example, we only note that the
media-change event is supported after some time passes -- not in the
initial slave_config. Or error handler may disable it at runtime
because that event is problematic.
As such, that implies that the LLDD (with help from scsi_lib) is
dynamically adding and removing these attributes at runtime -- a lot
more complexity than is really needed AFAICS.
It is easy and straightforward for the driver to set a bit.
We cannot assume the state of event support bits are constant from
modprobe/slave_config time.
Jeff
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
