Re: [PATCH 3/4] libmultipath: path latency: simplify getprio()

From: Martin Wilck <mwilck@suse.com>
To: Guan Junxiong <guanjunxiong@huawei.com>,
	Christophe Varoqui <christophe.varoqui@opensvc.com>
Cc: dm-devel@redhat.com, niuhaoxin <niuhaoxin@huawei.com>,
	"Shenhong (C)" <shenhong09@huawei.com>
Subject: Re: [PATCH 3/4] libmultipath: path latency: simplify getprio()
Date: Thu, 07 Dec 2017 16:56:41 +0100	[thread overview]
Message-ID: <1512662201.3207.25.camel@suse.com> (raw)
In-Reply-To: <19c4c292-3297-b602-36c4-0dde2c61e266@huawei.com>

On Thu, 2017-12-07 at 12:26 +0800, Guan Junxiong wrote:
> Hi Martin,
> 
> I have tested this patch and found something needed to be correct. My
> comments inline.
> 
> 
> > @@ -340,8 +323,14 @@ int getprio(struct path *pp, char *args,
> > unsigned int timeout)
> >  			  ".It is recommend to be set larger",
> >  			  pp->dev, base_num);
> >  
> > +	standard_deviation = sqrt((sum_squares - lg_toldelay *
> > lg_toldelay)
> > +				  / (io_num -1));
> >  
> 
> This assignment is wrong. It gets a "NAN" for standard_deviation.
> It should be the following equation according to sum_n (x_i -
> avg(x))^2 == sum_n x_i^2 - n * avg(x)^2
> standard_deviation = sqrt((sum_squares - io_num* lg_avglatency*
> lg_avglatency)/ (io_num -1));
>                    = sqrt((sum_squares -  lg_toldelay*
> lg_avglatency)/ (io_num -1));

That's of course correct. I'll submit an update asap.

> I still have the doubt about the computation of the "uncertainty"
> item.
> Because I have observed that the uncertainty is in the range of 1.3 ~
> 1.6 whenever
> the base_num varies from 1.1 to 10.

> Do you mean the uncertainty as the "Error function" of (log) normal
> distribution?

> Here is the definition of
> https://en.wikipedia.org/wiki/Error_function
> 
> I prefer to using  "Error function" that describes accumulated
> probability of  how  prio
> locates in the (-inf, prio-1) and (prio+1, +inf).

There's a misunderstanding here.

My "uncertainty factor" describes the width of the distribution of the
*measured* latencies. It roughly represents the accuracy of the
measurement (indicating that 68%, or error_function (sqrt(1/2)), of the
measured values lie in the interval [avg/F, avg*F]). IOW, it tells you
how stable your latencies for a certain path are.

It has nothing to do with the artificial prio "bins" that the latency
prioritizer sets up. Therefore it *has to be independent* of base_num.
It just gives an indication how large base_num should be. 

I hope this makes it clear.

Martin

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel