Date:	Thu, 26 Nov 2009 14:53:38 +0900 (JST)
From:	Hitoshi Mitake <mitake@....info.waseda.ac.jp>
To:	dvhltc@...ibm.com
Cc:	mingo@...e.hu, linux-kernel@...r.kernel.org, walken@...gle.com,
	a.p.zijlstra@...llo.nl, paulus@...ba.org, fweisbec@...il.com
Subject: Re: [PATCH 2/3] perf bench: Add new files for futex performance
 test

From: Darren Hart <dvhltc@...ibm.com>
Subject: Re: [PATCH 2/3] perf bench: Add new files for futex performance test
Date: Tue, 24 Nov 2009 08:33:16 -0800

Hi Darren,

> 
> Hitoshi Mitake wrote:
> > This patch adds two new files.
> > 
> 
> Hi Hitoshi-san,
> 
>  > futextest.h provides general wrappers for futex() system call.
>  > This patch containts the line:
>  > typedef volatile u_int32_t futex_t;
>  > I know that new typedef is not a thing to welcome,
>  > but this is useful thing.
>  >
>  > futex-wait.c is a new suite to test performance of FUTEX_WAIT.
>  >
>  > These files are from Darren Hart's futex test,
>  > and futex-wait.c is based on the program originally
>  > written by Michel Lespinasse.
> 
> Thanks for looking at getting the futex performance tests into perf. 
> Just wanted to make sure you are aware that there will likely be several 
> more futextest/performance tests in the near future as the project is 
> under early active development. I see you have merged harness.h into 
> futex-wait.c, which is fine, but I will likely be adding a new 
> include/locking.h to add things like barrier, lock, and lock_pi locking 
> primitives to the futextest testsuite. futex-wait.c would then be 
> updated to use those directly if feasible and remove the custom locking 
> primitives in the test itself. I mention this so you are aware and perf 
> futex benchmarks don't get too far out of sync with futextest.
> 
> I'd be interested in any ideas you have to make futextest/performance/* 
> tests integrate more easily into perf as I'd like to include each new 
> test into perf as well.
> 

There are two reasons why I changed and merged harness.h into futex-wait.c.
1) Thread numbers are fixed
2) Output style are fixed
But it seems that there is no other problem.
So I wrote the patch making locktest() more general.
I'll post it later in this thread, could you review this?

> Couple of nits below:
> 
> 
> > diff --git a/tools/perf/bench/futextest.h b/tools/perf/bench/futextest.h
> > new file mode 100644
> > index 0000000..09d8b94
> > --- /dev/null
> > +++ b/tools/perf/bench/futextest.h
> > @@ -0,0 +1,280 @@
> > +/******************************************************************************
> > + *
> > + *   Copyright B) International Business Machines  Corp., 2009
> 
> 
> Copyright character issue...

Sorry again...

> 
> > + * HISTORY
> > + *      2009-Nov-24:
> > + *       Ported to perf by Hitoshi Mitake <mitake@....info.waseda.ac.jp>
> > + *      2009-Nov-06: Initial version by Darren Hart <dvhltc@...ibm.com>
> 
> I usually add the dates in ascending chronological order.

Sorry, I'll fix it.

> 
> > + *
> > + *****************************************************************************/
> > +
> > +#ifndef _FUTEXTEST_H
> > +#define _FUTEXTEST_H
> > +
> > +#include <unistd.h>
> > +#include <sys/syscall.h>
> > +#include <sys/types.h>
> > +#include <linux/futex.h>
> > +
> > +typedef volatile u_int32_t futex_t;
> 
> I have been considering making this into a val wrapped in a struct like 
> the atomic_t as that will make adding things like flags to the futex_t 
> easier. Again, just a head's up that it may be changing in the near future.

Of course I'll chase your changes in the future :)

Thanks
	Hitoshi

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists