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

Arjan van de Ven
2005-12-15T07:37:12Z

On Thu, 2005-12-15 at 04:54 +0100, Martin Peschke wrote:
> Christoph Hellwig wrote:
>
> >>+ atomic_t read_num;
> >>+ atomic_t write_num;
> >>+ struct statistic_interface *stat_if;
> >>+ struct statistic *stat_sizes_scsi_write;
> >>+ struct statistic *stat_sizes_scsi_read;
> >>+ struct statistic *stat_sizes_scsi_nodata;
> >>+ struct statistic *stat_sizes_scsi_nofit;
> >>+ struct statistic *stat_sizes_scsi_nomem;
> >>+ struct statistic *stat_sizes_timedout_write;
> >>+ struct statistic *stat_sizes_timedout_read;
> >>+ struct statistic *stat_sizes_timedout_nodata;
> >>+ struct statistic *stat_latencies_scsi_write;
> >>+ struct statistic *stat_latencies_scsi_read;
> >>+ struct statistic *stat_latencies_scsi_nodata;
> >>+ struct statistic *stat_pending_scsi_write;
> >>+ struct statistic *stat_pending_scsi_read;
> >>+ struct statistic *stat_erp;
> >>+ struct statistic *stat_eh_reset;
> >>
> >>
> >
> >NACK. pretty much all of this is generic and doesn't belong into an LLDD.
> >We already had this statistics things with emulex and they added various
> >bits to the core in response.
> >
> >
> >
> >
> Agreed. It's not necessarily up to LLDDs to keep track of request sizes,
> request latencies, I/O queue utilization, and error recovery conditions
> by means of statistics. This could or maybe should be done in a more
> central spot.
>
> With regard to latencies, it might make some difference, though, how
> many layers are in between that cause additional delays. Then the
> question is which latency one wants to measure.

even if the LLDD measures these, the stats belong a level up, so that
all LLDD's export the same. I think you got half of Christophs point,
but not this last bit: even when it's the LLDD that needs to measure the
stat, it still shouldn't be LLDD specific, and thus defined one if not
two layers up.