lkml.org 
[lkml]   [2014]   [Dec]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v3] Perf Bench: Locking Microbenchmark
From
Date
On Thu, 2014-12-11 at 18:12 -0300, Arnaldo Carvalho de Melo wrote:
> > +struct timeval start, end, total;
> > +static unsigned int start_nr = 100;
> > +static unsigned int end_nr = 1100;
> > +static unsigned int increment_by = 100;
> > +static int bench_dur = NOTSET;
> > +static int num_jobs = NOTSET;
> > +static bool run_jobs;
> > +static int n_pro;
> > +
> > +/* Shared variables between fork processes*/
> > +unsigned int *finished, *setup;
> > +unsigned long long *shared_workers;
> > +char *tmp_dir;
>
> Are you sure these variables aren't static?
>

Yes you are right. I will change these to static.


> > +/* Running bench vfs workload */
> > +static void *run_bench_vfs(struct worker *workers)
> > +{
> > + int fd;
> > + unsigned long long nr_ops = 0;
> > + int jobs = num_jobs;
> > +
> > + sprintf(workers->str, "%s/%d-XXXXXX", tmp_dir, getpid());
>
>
> Please use snprintf, checking for overflows on the target string
>

Thank you for pointing this out. I will change all sprintf to use
snprintf and check for overflows on the target string.


> > +
> > + /* finished shared var is use to signal start and end of benchmark */
> > + finished_tmp = (void *)mmap(0, sizeof(unsigned int), PROT_READ|PROT_WRITE,
> > + MAP_SHARED|MAP_ANONYMOUS, -1, 0);
>
> Why do you use these void * casts before mmap alreayd returns void *?
>

Yes the cast to void here is redundant. I will remove the cast to (void
*) here.

> > + if (finished_tmp == (void *) -1)
>
> Please use MAP_FAILED instead of its equivalent (void *) -1.
>

I will check for MAP_FAILED here instead. Thank you for the suggestion.


-Tuan




\
 
 \ /
  Last update: 2014-12-18 00:01    [W:0.222 / U:0.052 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site