Re: Threaded file system benchmark

[Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>]

Ryo ONODERA <ryo_on%yk.rim.or.jp@localhost> wrote:
> ...
> 
> --- a/gettime.c
> +++ b/gettime.c
> @@ -483,18 +483,30 @@ static void *clock_thread_fn(void *data)
>  {
>  	struct clock_thread *t = data;
>  	struct clock_entry *c;
> +#if defined(__NetBSD__)
> +	os_cpu_mask_t *cpu_mask = cpuset_create();
> +#else
>  	os_cpu_mask_t cpu_mask;
> +#endif

You can always use a pointer, e.g.:

#if defined(__NetBSD__)
	os_cpu_mask_t *cpu_mask = cpuset_create();
#else
	os_cpu_mask_t cpu_mask_store;
	os_cpu_mask_t *cpu_mask = &cpu_mask_store;
#endif

This will save some #ifdef __NetBSD__ elsewhere.

> --- a/hash.h
> +++ b/hash.h
> @@ -18,6 +18,10 @@
>   * machines where multiplications are slow.
>   */
>  
> +#if !defined(BITS_PER_LONG)
> +#define	BITS_PER_LONG __SIZEOF_LONG__*8
> +#endif

I did not look how exactly is this used, but it is a good idea to use
brackets around the expression when dealing with macros.  Just prevents
from any future bugs.

> +static inline int fio_setaffinity(int tid, os_cpu_mask_t *cpumask)
> +{
> +	return _sched_getaffinity(getpid(), tid, cpuset_size(cpumask), cpumask);
> +}

I suppose you meant _sched_setaffinity() here?  Well, that is your bug.
Also, _sched_{get,set}affinity() are internal calls which should not be
used as a public API.  Instead, pthread_{get,set}affinity_np() should
be used.  See affinity(3) man page for the details.

> --- a/parse.c
> +++ b/parse.c
> @@ -152,7 +152,7 @@ static unsigned long long get_mult_time(const char
> *str, int len, 
>  	c = strdup(p);
>  	for (int i = 0; i < strlen(c); i++)
> -		c[i] = tolower(c[i]);
> +		c[i] = tolower((int)c[i]);

The value should be type casted to unsigned char rather than int.
See CAVEATS section of the ctype(3) man page for the explanation.

-- 
Mindaugas