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

Martin Peschke
2005-12-15T11:23:58Z

Arjan van de Ven wrote:

>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.
>
>
>

Ah, I see. It makes sense to avoid multiple places where to look for
latencies, for example.
Several ways to accomplish this come to mind:

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. This causes a proliferation of
such functions (one upper layer function per statistic class). Since
control goes back and force between upper and lower layer drivers
anyway, adding another call to the backchain doesn't seem to be the
most efficient way. Not sure an addional indirect function call to the
layer actually owning a particular statistic could be avoided in any
case (depends on interface between the two layers).

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.