Re: [PATCH] oslat: Add command line option for bucket width

[John Kacur <jkacur@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2272 bytes --]



On Fri, 9 Dec 2022, Crystal Wood wrote:

> On Fri, 2022-12-09 at 20:03 -0500, John Kacur wrote:
> > 
> > 
> > On Thu, 8 Dec 2022, Crystal Wood wrote:
> > 
> > 
> > >  
> > > @@ -677,6 +699,20 @@ static void parse_options(int argc, char *argv[])
> > >                                 exit(1);
> > >                         }
> > >                         break;
> > > +               case OPT_BUCKETWIDTH:
> > > +               case 'W':
> > > +                       g.bucket_width = strtol(optarg, NULL, 10);
> > > +                       if (g.bucket_size <= 0) {
> > 
> > I think this should be g.bucket_width
> 
> Oops
> 
> > 
> > A quick first look through and run, see the one comment above
> > near "case 'W'"
> > 
> > and then
> > 
> > checkpatch reports some minor easily fixed problems
> > 
> > ../linux/scripts/checkpatch.pl oslat.patch 
> > ERROR: code indent should use tabs where possible
> > #100: FILE: src/oslat/oslat.c:342:
> > +^I^I          g.precision, us);$
> > 
> > ERROR: code indent should use tabs where possible
> > #102: FILE: src/oslat/oslat.c:344:
> > +^I^I         g.precision, us);$
> 
> I was matching the existing style in the file that tended to use spaces for
> alignment.
> 
> > ERROR: spaces required around that '=' (ctx:VxV)
> > #227: FILE: src/oslat/oslat.c:654:
> > +       OPT_BUCKETSIZE=1, OPT_BUCKETWIDTH, OPT_CPU_LIST, 
> > OPT_CPU_MAIN_THREAD,
> >                       ^
> 
> I only added OPT_BUCKETWIDTH to the list; I didn't touch the =1 part.
> 
> -Scott
> 
> 

One more thing that we just saw, if you run oslat without any options, 
just the defaults you get 32 buckets with the highest one being 32us

But if you run
oslat -W 500

You still get 32 buckets but since the width is half, then largest bucket
is 32us

This increases the resolution of the buckets, but it puts all the overflow
in the 16us buckets, wondering if we should double the number of buckets
so that the largest one is still 32us ?

I realize you could do
oslat -b 64  -W 500
to achieve that, but perhaps the default is not good like this.

John