Re: [patch 6/6] statistics infrastructure - exploitation: zfcp

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

 



Arjan van de Ven wrote:

On Thu, 2005-12-15 at 12:23 +0100, Martin Peschke wrote:

Given the idea of struct statistic, the lower layer driver could use a given pointer to an upper layer's struct statistic in order to call statistic_inc(stat, x).

The lower layer driver could call an upper layer driver's function to have the upper layer update a statistic.

Why? it's an open source world, what you suggest is more something for a
"must hide behind interfaces" closed world ;)
Regarding the statistic_inc() & friends proposal, I see it as a handy abstraction that allows device driver programmers to worry about other things than reimplementing counters and other vehicles conveying statistics information. If statistic_inc() would only be able to hide a single counter the value of which is just increased over time, I would agree with you. But it implements several ways of data processing, including counters, fill level indicators (counters for total number of measurements, plus minimum, average, maximum), histograms (a set of counters for discrete values or ranges of values), as well as statistics that take the dimension time into account (for things like megabytes per seconds, or queue utilization per whatever-unit-of-time).

Regarding the "lower layer driver could call an upper layer driver's function" idea: I didn't say I like all the listed alternatives the same. Actually I don't like this one. I just wanted to be polite and discuss alternatives in order not to appear to be ignorant by simply insisting on my approach ;)

if done right, the LLDD gets access to the transport class information,
including the array of stats, so the LLDD can update those just fine.
Just the API should be clear about who owns updating which field; a
comment will suffice for that ;)
Well, transport classes are fine for transport specific purposes.
I don't think that any statistic, particularly not request latencies and request sizes, belong there.

But I like the general idea hidden in what you say:
If done right, the layer that gathers statistics data gets access to the statistic, so that it can update this just fine.

The lower layer driver could temporarily store some measurement data in the data structure passed between those two; the upper layer driver picks it up later and calls whatever statistic library routine is appropriate. Requires additional bytes and one store/retrieve operation more than the struct statistic idea.

way way too complex for no reason.
Agreed, another non-preferred way of doing it.

Remember the scsi layer is a layered concept, but also upside down: even
though the transport class layers on top of the LLDD, it's the LLDD that
drives that class, not the other way around. The same could be done with
selected statistics; have the transport layer do the exporting to sysfs
and all that stuff, but have the LLDD keep track of them.

That's one of the major points of my patch: Have the statistics library do the exporting to the user interface, not the LLDD.

Debugfs has been my initial choice instead of sysfs because I don't think sysfs appropriate for exporting histograms and stuff. But whether debugfs is the right choice, or relayfs, or sysfs, or something else is another question, I'd like to find an answer for in this thread.

If you are interested in the user interface question, this is a sample of what some statistics currently look like in a debugfs file:

 latencies_scsi_write <=0 0
 latencies_scsi_write <=1 0
 latencies_scsi_write <=2 0
 latencies_scsi_write <=4 174
 latencies_scsi_write <=8 872
 latencies_scsi_write <=16 2555
 latencies_scsi_write <=32 2483
 ...
 latencies_scsi_write <=1024 1872
 latencies_scsi_write >1024 1637

 latencies_scsi_read <=0 0
 latencies_scsi_read <=1 0
 latencies_scsi_read <=2 0
 latencies_scsi_read <=4 57265
 latencies_scsi_read <=8 13610
 latencies_scsi_read <=16 1082
 latencies_scsi_read <=32 319
 latencies_scsi_read <=64 63
 ...
 latencies_scsi_read >1024 0

 ...
 util_qdio_outb [3097394.211992] 865 1 1.052 5
 util_qdio_outb [3097395.211992] 737 1 4.558 125
 util_qdio_outb [3097396.211992] 396 1 11.765 77
 util_qdio_outb [3097397.211992] 270 1 12.863 128
 util_qdio_outb [3097398.211992] 765 1 7.271 26
 util_qdio_outb [3097399.211992] 577 1 4.036 27
 ...

(of course
only for those that are relevant in this sense; if the transport class
is the natural place to update, then it should be done there)
yes

Martin

-
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]
  Powered by Linux