Hi,
From: Mindaugas Rasiukevicius <rm...@netbsd.org>, Date: Sun, 12 Jul 2015
15:13:56 +0100
> Ryo ONODERA <ryo...@yk.rim.or.jp> 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.
I will reduce #ifdef __NetBSD__.
>> --- 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.
I will use "(__SIZEOF_LONG__*CHAR_BIT)".
>> +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.
It may be the real cause of my problem.
Thank you very much.
I understand that internal calls should not be used.
I read affinity(3) man page, and found Solaris/HP-UX compatible pset(3)
APIs.
I will use pset(3), I may borrow the code from Solaris's definitions.
>> --- 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.
I see.
I have added this to pass "gcc -Werror", however fio does not require
-Werror originally. I will remove it.
> --
> Mindaugas
Thank you very much.
--
Ryo ONODERA // ryo...@yk.rim.or.jp
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB FD1B F404 27FA C7D1 15F3
