Avoid of using /proc/cpuinfo in generic tests

Avoid of using /proc/cpuinfo in generic tests

[Stanislav Kholmanskikh]

Hi!

The content of /proc/cpuinfo is platform dependent. I think It
would be better to use sysfs interface.

Thanks.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 1/2] xfstests: implemented _no_of_online_cpus() function

[Stanislav Kholmanskikh]

Its purpose is to get a number of online cpus.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 common/rc |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/common/rc b/common/rc
index 77e96c4..ff2f7fe 100644
--- a/common/rc
+++ b/common/rc
@@ -2145,6 +2145,25 @@ _scale_fsstress_args()
     echo $args
 }
 
+_no_of_online_cpus()
+{
+	count=0
+
+	for i in `cat /sys/devices/system/cpu/online | $SED_PROG 's/,/ /g'`; do
+		count=$(( $count + 1 ))
+
+		lnumber=`echo $i | cut -d '-' -f 1`
+		rnumber=`echo $i | cut -d '-' -f 2`
+
+		while [ $lnumber -lt $rnumber ]; do
+			lnumber=$(( $lnumber + 1 ))
+			count=$(( $count + 1 ))
+		done
+	done
+
+	echo $count
+}
+
 run_check()
 {
 	echo "# $@" >> $seqres.full 2>&1
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/2] xfstests: generic/273: do not use /proc/cpuinfo

[Stanislav Kholmanskikh]

The content of /proc/cpuinfo file is platform-dependent.
So we can not use it reliably to check a number of available cpus.
It would be better to use sysfs interface, as _no_of_online_cpus() does.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 tests/generic/273 |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tests/generic/273 b/tests/generic/273
index d149808..c028438 100755
--- a/tests/generic/273
+++ b/tests/generic/273
@@ -47,7 +47,7 @@ count=2
 
 _threads_set()
 {
-	_cpu_num=`cat /proc/cpuinfo | grep "processor" | wc -l`
+	_cpu_num=`_no_of_online_cpus`
 	threads=$(($_cpu_num * 50))
 	if [ $threads -gt 200 ]
 	then
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/2] xfstests: implemented _no_of_online_cpus() function

[Carlos Maiolino]

This looks good to me.

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

On Wed, Oct 23, 2013 at 03:58:43PM +0400, Stanislav Kholmanskikh wrote:
> Its purpose is to get a number of online cpus.
> 
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
>  common/rc |   19 +++++++++++++++++++
>  1 files changed, 19 insertions(+), 0 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index 77e96c4..ff2f7fe 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2145,6 +2145,25 @@ _scale_fsstress_args()
>      echo $args
>  }
>  
> +_no_of_online_cpus()
> +{
> +	count=0
> +
> +	for i in `cat /sys/devices/system/cpu/online | $SED_PROG 's/,/ /g'`; do
> +		count=$(( $count + 1 ))
> +
> +		lnumber=`echo $i | cut -d '-' -f 1`
> +		rnumber=`echo $i | cut -d '-' -f 2`
> +
> +		while [ $lnumber -lt $rnumber ]; do
> +			lnumber=$(( $lnumber + 1 ))
> +			count=$(( $count + 1 ))
> +		done
> +	done
> +
> +	echo $count
> +}
> +
>  run_check()
>  {
>  	echo "# $@" >> $seqres.full 2>&1
> -- 
> 1.7.1
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

-- 
Carlos

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/2] xfstests: generic/273: do not use /proc/cpuinfo

[Carlos Maiolino]

Looks good indeed.

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
On Wed, Oct 23, 2013 at 03:58:44PM +0400, Stanislav Kholmanskikh wrote:
> The content of /proc/cpuinfo file is platform-dependent.
> So we can not use it reliably to check a number of available cpus.
> It would be better to use sysfs interface, as _no_of_online_cpus() does.
> 
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
>  tests/generic/273 |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/tests/generic/273 b/tests/generic/273
> index d149808..c028438 100755
> --- a/tests/generic/273
> +++ b/tests/generic/273
> @@ -47,7 +47,7 @@ count=2
>  
>  _threads_set()
>  {
> -	_cpu_num=`cat /proc/cpuinfo | grep "processor" | wc -l`
> +	_cpu_num=`_no_of_online_cpus`
>  	threads=$(($_cpu_num * 50))
>  	if [ $threads -gt 200 ]
>  	then
> -- 
> 1.7.1
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

-- 
Carlos

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/2] xfstests: implemented _no_of_online_cpus() function

[Dave Chinner]

On Wed, Oct 23, 2013 at 03:58:43PM +0400, Stanislav Kholmanskikh wrote:
> Its purpose is to get a number of online cpus.
> 
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
>  common/rc |   19 +++++++++++++++++++
>  1 files changed, 19 insertions(+), 0 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index 77e96c4..ff2f7fe 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2145,6 +2145,25 @@ _scale_fsstress_args()
>      echo $args
>  }
>  
> +_no_of_online_cpus()
> +{
> +	count=0
> +
> +	for i in `cat /sys/devices/system/cpu/online | $SED_PROG 's/,/ /g'`; do
> +		count=$(( $count + 1 ))

That's just as kernel/platform dependent as using /proc/cpuinfo.

Indeed, note this in generic/273:

_supported_os IRIX Linux

It's supposed to work on Irix as well, which means using sysfs is
just as wrong as using /proc/cpuinfo.

sysconf is the prefered platform neutral interface for getting this
sort of information. Something as simple as:

$ getconf _NPROCESSORS_ONLN
8
$

or adding a sysconf(_SC_NPROCESSORS_ONLN) call to src/features.c (if
we don't want to rely on systems having getconf installed) would be
much better than manually parsing magic proc or sysfs files.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/2] xfstests: generic/273: do not use /proc/cpuinfo

[Rich Johnston]

This has been committed.

Thanks
--Rich

commit fd080d64b6e9677cb9d0a15dc7e308b6ca0e273f
Author: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
Date:   Wed Oct 23 11:58:44 2013 +0000

     xfstests: generic/273: do not use /proc/cpuinfo

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/2] xfstests: generic/273: do not use /proc/cpuinfo

[Dave Chinner]

On Wed, Oct 23, 2013 at 07:14:40PM -0500, Rich Johnston wrote:
> This has been committed.

Without commiting the other patch that implements
_no_of_online_cpus....

> Thanks
> --Rich
> 
> commit fd080d64b6e9677cb9d0a15dc7e308b6ca0e273f
> Author: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> Date:   Wed Oct 23 11:58:44 2013 +0000
> 
>     xfstests: generic/273: do not use /proc/cpuinfo

generic/273 22s ... - output mismatch (see /home/dave/src/xfstests-dev/results//generic/273.out.bad)
    --- tests/generic/273.out   2013-03-28 07:53:08.000000000 +1100
    +++ /home/dave/src/xfstests-dev/results//generic/273.out.bad        2013-10-24 12:06:27.000000000 +1100
    @@ -2,3 +2,5 @@
     ------------------------------
     start the workload
     ------------------------------
    +./tests/generic/273: line 50: _no_of_online_cpus: command not found
    +./tests/generic/273: line 51: * 50: syntax error: operand expected (error token is "* 50")
     ...
     (Run 'diff -u tests/generic/273.out /home/dave/src/xfstests-dev/results//generic/273.out.bad' to see the entire diff)
Ran: generic/273
Failures: generic/273
Failed 1 of 1 tests

Rich, can you try to commit patch series as a whole, not piecemeal
while parts of the patch series are still being discussed and
reviewed?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH] xfstests: src/feature.c: print a number of online CPUs

[Stanislav Kholmanskikh]

For this purpose we use sysconf() as it is the
preferred platform neutral interface for getting this
sort of information.

Based on Dave Chinner proposal.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 src/feature.c |   31 ++++++++++++++++++++++++++++---
 1 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/src/feature.c b/src/feature.c
index 2619ca5..a3ce9b8 100644
--- a/src/feature.c
+++ b/src/feature.c
@@ -30,6 +30,7 @@
  * Return code: 0 is true, anything else is error/not supported
  *
  * Test for machine features
+ *   -o  report a number of online cpus
  *   -s  report pagesize
  *   -w  report bits per long
  */
@@ -39,6 +40,7 @@
 #include <sys/quota.h>
 #include <sys/resource.h>
 #include <signal.h>
+#include <unistd.h>
 
 #ifdef HAVE_XFS_XQM_H
 #include <xfs/xqm.h>
@@ -64,7 +66,7 @@ usage(void)
 	fprintf(stderr, "Usage: feature [-v] -<q|u|g|p|U|G|P> <filesystem>\n");
 	fprintf(stderr, "       feature [-v] -c <file>\n");
 	fprintf(stderr, "       feature [-v] -t <file>\n");
-	fprintf(stderr, "       feature -s | -w\n");
+	fprintf(stderr, "       feature -o | -s | -w\n");
 	exit(1);
 }
 
@@ -212,9 +214,10 @@ main(int argc, char **argv)
 	int	uflag = 0;
 	int	Uflag = 0;
 	int	wflag = 0;
+	int	oflag = 0;
 	char	*fs = NULL;
 
-	while ((c = getopt(argc, argv, "ctgGpPqsuUvw")) != EOF) {
+	while ((c = getopt(argc, argv, "ctgGopPqsuUvw")) != EOF) {
 		switch (c) {
 		case 'c':
 			cflag++;
@@ -228,6 +231,9 @@ main(int argc, char **argv)
 		case 'G':
 			Gflag++;
 			break;
+		case 'o':
+			oflag++;
+			break;
 		case 'p':
 			pflag++;
 			break;
@@ -262,7 +268,7 @@ main(int argc, char **argv)
 		if (optind != argc-1)	/* need a device */
 			usage();
 		fs = argv[argc-1];
-	} else if (wflag || sflag) {
+	} else if (wflag || sflag || oflag) {
 		if (optind != argc)
 			usage();
 	} else 
@@ -306,6 +312,25 @@ bozo!
 #endif
 		exit(0);
 	}
+	if (oflag) {
+		long ncpus;
+
+#if defined(_SC_NPROCESSORS_ONLN)
+		/* Linux */
+		ncpus = sysconf(_SC_NPROCESSORS_ONLN);
+#elif defined(_SC_NPROC_ONLN)
+		/* IRIX */
+		ncpus = sysconf(_SC_NPROC_ONLN);
+#else
+		ncpus = 0;
+#endif
+		if (ncpus == -1)
+			ncpus = 0;
+
+		printf("%ld\n", ncpus);
+
+		exit(0);
+	}
 
 	fprintf(stderr, "feature: dunno what you're after.\n");
 	return(1);
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfstests: src/feature.c: print a number of online CPUs

[Dave Chinner]

On Thu, Oct 24, 2013 at 12:56:38PM +0400, Stanislav Kholmanskikh wrote:
> For this purpose we use sysconf() as it is the
> preferred platform neutral interface for getting this
> sort of information.
> 
> Based on Dave Chinner proposal.
> 
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
>  src/feature.c |   31 ++++++++++++++++++++++++++++---
>  1 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/src/feature.c b/src/feature.c
> index 2619ca5..a3ce9b8 100644
> --- a/src/feature.c
> +++ b/src/feature.c
> @@ -30,6 +30,7 @@
>   * Return code: 0 is true, anything else is error/not supported
>   *
>   * Test for machine features
> + *   -o  report a number of online cpus
>   *   -s  report pagesize
>   *   -w  report bits per long
>   */
> @@ -39,6 +40,7 @@
>  #include <sys/quota.h>
>  #include <sys/resource.h>
>  #include <signal.h>
> +#include <unistd.h>
>  
>  #ifdef HAVE_XFS_XQM_H
>  #include <xfs/xqm.h>
> @@ -64,7 +66,7 @@ usage(void)
>  	fprintf(stderr, "Usage: feature [-v] -<q|u|g|p|U|G|P> <filesystem>\n");
>  	fprintf(stderr, "       feature [-v] -c <file>\n");
>  	fprintf(stderr, "       feature [-v] -t <file>\n");
> -	fprintf(stderr, "       feature -s | -w\n");
> +	fprintf(stderr, "       feature -o | -s | -w\n");
>  	exit(1);
>  }
>  
> @@ -212,9 +214,10 @@ main(int argc, char **argv)
>  	int	uflag = 0;
>  	int	Uflag = 0;
>  	int	wflag = 0;
> +	int	oflag = 0;
>  	char	*fs = NULL;
>  
> -	while ((c = getopt(argc, argv, "ctgGpPqsuUvw")) != EOF) {
> +	while ((c = getopt(argc, argv, "ctgGopPqsuUvw")) != EOF) {
>  		switch (c) {
>  		case 'c':
>  			cflag++;
> @@ -228,6 +231,9 @@ main(int argc, char **argv)
>  		case 'G':
>  			Gflag++;
>  			break;
> +		case 'o':
> +			oflag++;
> +			break;
>  		case 'p':
>  			pflag++;
>  			break;
> @@ -262,7 +268,7 @@ main(int argc, char **argv)
>  		if (optind != argc-1)	/* need a device */
>  			usage();
>  		fs = argv[argc-1];
> -	} else if (wflag || sflag) {
> +	} else if (wflag || sflag || oflag) {
>  		if (optind != argc)
>  			usage();
>  	} else 
> @@ -306,6 +312,25 @@ bozo!
>  #endif
>  		exit(0);
>  	}
> +	if (oflag) {
> +		long ncpus;
> +
> +#if defined(_SC_NPROCESSORS_ONLN)
> +		/* Linux */
> +		ncpus = sysconf(_SC_NPROCESSORS_ONLN);
> +#elif defined(_SC_NPROC_ONLN)
> +		/* IRIX */
> +		ncpus = sysconf(_SC_NPROC_ONLN);
> +#else
> +		ncpus = 0;
> +#endif
> +		if (ncpus == -1)
> +			ncpus = 0;
> +
> +		printf("%ld\n", ncpus);

Actually, I'd say we shoul default to 1 cpu if we can't get the
number of CPUs. Clearly we have at least one if we can run this
code. :)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH V2] xfstests: src/feature.c: print a number of online CPUs

[Stanislav Kholmanskikh]

For this purpose we use sysconf() as it is the
preferred platform neutral interface for getting this
sort of information.

Based on Dave Chinner proposal.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 src/feature.c |   31 ++++++++++++++++++++++++++++---
 1 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/src/feature.c b/src/feature.c
index 2619ca5..a3ce9b8 100644
--- a/src/feature.c
+++ b/src/feature.c
@@ -30,6 +30,7 @@
  * Return code: 0 is true, anything else is error/not supported
  *
  * Test for machine features
+ *   -o  report a number of online cpus
  *   -s  report pagesize
  *   -w  report bits per long
  */
@@ -39,6 +40,7 @@
 #include <sys/quota.h>
 #include <sys/resource.h>
 #include <signal.h>
+#include <unistd.h>
 
 #ifdef HAVE_XFS_XQM_H
 #include <xfs/xqm.h>
@@ -64,7 +66,7 @@ usage(void)
 	fprintf(stderr, "Usage: feature [-v] -<q|u|g|p|U|G|P> <filesystem>\n");
 	fprintf(stderr, "       feature [-v] -c <file>\n");
 	fprintf(stderr, "       feature [-v] -t <file>\n");
-	fprintf(stderr, "       feature -s | -w\n");
+	fprintf(stderr, "       feature -o | -s | -w\n");
 	exit(1);
 }
 
@@ -212,9 +214,10 @@ main(int argc, char **argv)
 	int	uflag = 0;
 	int	Uflag = 0;
 	int	wflag = 0;
+	int	oflag = 0;
 	char	*fs = NULL;
 
-	while ((c = getopt(argc, argv, "ctgGpPqsuUvw")) != EOF) {
+	while ((c = getopt(argc, argv, "ctgGopPqsuUvw")) != EOF) {
 		switch (c) {
 		case 'c':
 			cflag++;
@@ -228,6 +231,9 @@ main(int argc, char **argv)
 		case 'G':
 			Gflag++;
 			break;
+		case 'o':
+			oflag++;
+			break;
 		case 'p':
 			pflag++;
 			break;
@@ -262,7 +268,7 @@ main(int argc, char **argv)
 		if (optind != argc-1)	/* need a device */
 			usage();
 		fs = argv[argc-1];
-	} else if (wflag || sflag) {
+	} else if (wflag || sflag || oflag) {
 		if (optind != argc)
 			usage();
 	} else 
@@ -306,6 +312,25 @@ bozo!
 #endif
 		exit(0);
 	}
+	if (oflag) {
+		long ncpus;
+
+#if defined(_SC_NPROCESSORS_ONLN)
+		/* Linux */
+		ncpus = sysconf(_SC_NPROCESSORS_ONLN);
+#elif defined(_SC_NPROC_ONLN)
+		/* IRIX */
+		ncpus = sysconf(_SC_NPROC_ONLN);
+#else
+		ncpus = 1;
+#endif
+		if (ncpus == -1)
+			ncpus = 1;
+
+		printf("%ld\n", ncpus);
+
+		exit(0);
+	}
 
 	fprintf(stderr, "feature: dunno what you're after.\n");
 	return(1);
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfstests: src/feature.c: print a number of online CPUs

[Carlos Maiolino]

Hi Dave,

I'm not sure about setting the default to 1 cpu might me a good behavior. My
apologies if I'm saying something wrong, but, if the 'tester' are trying to do
some test trusting on the amount of cpus, it might not be a good behavior.
I was thinking, how about issue an error message if xfstests can't properly
detect the amount of cpus from the system, and add any kind of usage option to
specify the numbers of cpus? So in case of a error while detecting the amount of
cpus.


> 
> Actually, I'd say we shoul default to 1 cpu if we can't get the
> number of CPUs. Clearly we have at least one if we can run this
> code. :)
> 
> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

-- 
Carlos

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfstests: src/feature.c: print a number of online CPUs

[Stanislav Kholmanskikh]

On 10/24/2013 05:18 PM, Carlos Maiolino wrote:
> Hi Dave,
>
> I'm not sure about setting the default to 1 cpu might me a good behavior. My
> apologies if I'm saying something wrong, but, if the 'tester' are trying to do
> some test trusting on the amount of cpus, it might not be a good behavior.
> I was thinking, how about issue an error message if xfstests can't properly
> detect the amount of cpus from the system, and add any kind of usage option to
> specify the numbers of cpus? So in case of a error while detecting the amount of
> cpus.
What about this:

If we can not determine the number of available cpus we just
print '-1' (as sysconf() returns in that case) and exit with non-zero 
status.
And verify the exit status in the test cases.

What do you mean by:
"and add any kind of usage option to specify the numbers of cpus"
?

Thanks.

>
>
>> Actually, I'd say we shoul default to 1 cpu if we can't get the
>> number of CPUs. Clearly we have at least one if we can run this
>> code. :)
>>
>> Cheers,
>>
>> Dave.
>> -- 
>> Dave Chinner
>> david@fromorbit.com
>>
>> _______________________________________________
>> xfs mailing list
>> xfs@oss.sgi.com
>> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfstests: src/feature.c: print a number of online CPUs

[Dave Chinner]

[ insert comment about not top-posting on mainling lists ]

On Thu, Oct 24, 2013 at 11:18:01AM -0200, Carlos Maiolino wrote:
> > Actually, I'd say we shoul default to 1 cpu if we can't get the
> > number of CPUs. Clearly we have at least one if we can run this
> > code. :)
>
> I'm not sure about setting the default to 1 cpu might me a good behavior. My
> apologies if I'm saying something wrong, but, if the 'tester' are trying to do
> some test trusting on the amount of cpus, it might not be a good behavior.
> I was thinking, how about issue an error message if xfstests can't properly
> detect the amount of cpus from the system, and add any kind of usage option to
> specify the numbers of cpus? So in case of a error while detecting the amount of
> cpus.

I'd much prefer the test runs with a single CPU as a default rather
than not run at all. Most systems the tests run on support these
sysconf parameters, so it's going to do what we expect, but quite
frankly most tests shoul dnot need to know the number of CPUs.

This one is probably misguided, anyway, in what it's doing - if we
want to scale the load the test generates, then that's what
$LOAD_FACTOR is for. Also, it' multiplies the number of CPUs by 50,
then caps the result at 200, so in reality it's only scaling for up
to 4 CPUs which doesn't really take into account the range of
machines that we test on.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfstests: src/feature.c: print a number of online CPUs

[Stanislav Kholmanskikh]

On 10/25/2013 01:23 AM, Dave Chinner wrote:
> [ insert comment about not top-posting on mainling lists ]
>
> On Thu, Oct 24, 2013 at 11:18:01AM -0200, Carlos Maiolino wrote:
>>> Actually, I'd say we shoul default to 1 cpu if we can't get the
>>> number of CPUs. Clearly we have at least one if we can run this
>>> code. :)
>> I'm not sure about setting the default to 1 cpu might me a good behavior. My
>> apologies if I'm saying something wrong, but, if the 'tester' are trying to do
>> some test trusting on the amount of cpus, it might not be a good behavior.
>> I was thinking, how about issue an error message if xfstests can't properly
>> detect the amount of cpus from the system, and add any kind of usage option to
>> specify the numbers of cpus? So in case of a error while detecting the amount of
>> cpus.
> I'd much prefer the test runs with a single CPU as a default rather
> than not run at all. Most systems the tests run on support these
> sysconf parameters, so it's going to do what we expect, but quite
> frankly most tests shoul dnot need to know the number of CPUs.
>
> This one is probably misguided, anyway, in what it's doing - if we
> want to scale the load the test generates, then that's what
> $LOAD_FACTOR is for. Also, it' multiplies the number of CPUs by 50,
> then caps the result at 200, so in reality it's only scaling for up
> to 4 CPUs which doesn't really take into account the range of
> machines that we test on.

Hi!

Carlos, Dave, so what is the final resolution regarding my patch?

Thank you.

> Cheers,
>
> Dave.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfstests: src/feature.c: print a number of online CPUs

[Carlos Maiolino]

sysconf patch looks ok to me, but, I've never worked with sysconf myself, so,
although the patch looks good for me, I'd prefer Dave to take a deeper look on
it and see if he spot any problem here.

Cheers,

On Mon, Oct 28, 2013 at 11:23:52AM +0400, Stanislav Kholmanskikh wrote:
> 
> On 10/25/2013 01:23 AM, Dave Chinner wrote:
> >[ insert comment about not top-posting on mainling lists ]
> >
> >On Thu, Oct 24, 2013 at 11:18:01AM -0200, Carlos Maiolino wrote:
> >>>Actually, I'd say we shoul default to 1 cpu if we can't get the
> >>>number of CPUs. Clearly we have at least one if we can run this
> >>>code. :)
> >>I'm not sure about setting the default to 1 cpu might me a good behavior. My
> >>apologies if I'm saying something wrong, but, if the 'tester' are trying to do
> >>some test trusting on the amount of cpus, it might not be a good behavior.
> >>I was thinking, how about issue an error message if xfstests can't properly
> >>detect the amount of cpus from the system, and add any kind of usage option to
> >>specify the numbers of cpus? So in case of a error while detecting the amount of
> >>cpus.
> >I'd much prefer the test runs with a single CPU as a default rather
> >than not run at all. Most systems the tests run on support these
> >sysconf parameters, so it's going to do what we expect, but quite
> >frankly most tests shoul dnot need to know the number of CPUs.
> >
> >This one is probably misguided, anyway, in what it's doing - if we
> >want to scale the load the test generates, then that's what
> >$LOAD_FACTOR is for. Also, it' multiplies the number of CPUs by 50,
> >then caps the result at 200, so in reality it's only scaling for up
> >to 4 CPUs which doesn't really take into account the range of
> >machines that we test on.
> 
> Hi!
> 
> Carlos, Dave, so what is the final resolution regarding my patch?
> 
> Thank you.
> 
> >Cheers,
> >
> >Dave.
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

-- 
Carlos

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH V2] xfstests: src/feature.c: print a number of online CPUs

[Dave Chinner]

On Thu, Oct 24, 2013 at 04:10:40PM +0400, Stanislav Kholmanskikh wrote:
> For this purpose we use sysconf() as it is the
> preferred platform neutral interface for getting this
> sort of information.
> 
> Based on Dave Chinner proposal.
> 
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
....
> +	if (oflag) {
> +		long ncpus;
> +
> +#if defined(_SC_NPROCESSORS_ONLN)
> +		/* Linux */
> +		ncpus = sysconf(_SC_NPROCESSORS_ONLN);
> +#elif defined(_SC_NPROC_ONLN)
> +		/* IRIX */
> +		ncpus = sysconf(_SC_NPROC_ONLN);
> +#else
> +		ncpus = 1;
> +#endif
> +		if (ncpus == -1)
> +			ncpus = 1;

That can be simplified, and there's probably not much point in
listing the OS's that the different calls are for.

	if (oflag) {
		long ncpus = -1;

#if defined(_SC_NPROCESSORS_ONLN)
		ncpus = sysconf(_SC_NPROCESSORS_ONLN);
#elif defined(_SC_NPROC_ONLN)
		ncpus = sysconf(_SC_NPROC_ONLN);
#endif
		if (ncpus == -1)
			ncpus = 1;

		....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH V3] xfstests: src/feature.c: print a number of online CPUs

[Stanislav Kholmanskikh]

For this purpose we use sysconf() as it is the
preferred platform neutral interface for getting this
sort of information.

Based on Dave Chinner proposal.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 src/feature.c |   27 ++++++++++++++++++++++++---
 1 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/src/feature.c b/src/feature.c
index 2619ca5..334063b 100644
--- a/src/feature.c
+++ b/src/feature.c
@@ -30,6 +30,7 @@
  * Return code: 0 is true, anything else is error/not supported
  *
  * Test for machine features
+ *   -o  report a number of online cpus
  *   -s  report pagesize
  *   -w  report bits per long
  */
@@ -39,6 +40,7 @@
 #include <sys/quota.h>
 #include <sys/resource.h>
 #include <signal.h>
+#include <unistd.h>
 
 #ifdef HAVE_XFS_XQM_H
 #include <xfs/xqm.h>
@@ -64,7 +66,7 @@ usage(void)
 	fprintf(stderr, "Usage: feature [-v] -<q|u|g|p|U|G|P> <filesystem>\n");
 	fprintf(stderr, "       feature [-v] -c <file>\n");
 	fprintf(stderr, "       feature [-v] -t <file>\n");
-	fprintf(stderr, "       feature -s | -w\n");
+	fprintf(stderr, "       feature -o | -s | -w\n");
 	exit(1);
 }
 
@@ -212,9 +214,10 @@ main(int argc, char **argv)
 	int	uflag = 0;
 	int	Uflag = 0;
 	int	wflag = 0;
+	int	oflag = 0;
 	char	*fs = NULL;
 
-	while ((c = getopt(argc, argv, "ctgGpPqsuUvw")) != EOF) {
+	while ((c = getopt(argc, argv, "ctgGopPqsuUvw")) != EOF) {
 		switch (c) {
 		case 'c':
 			cflag++;
@@ -228,6 +231,9 @@ main(int argc, char **argv)
 		case 'G':
 			Gflag++;
 			break;
+		case 'o':
+			oflag++;
+			break;
 		case 'p':
 			pflag++;
 			break;
@@ -262,7 +268,7 @@ main(int argc, char **argv)
 		if (optind != argc-1)	/* need a device */
 			usage();
 		fs = argv[argc-1];
-	} else if (wflag || sflag) {
+	} else if (wflag || sflag || oflag) {
 		if (optind != argc)
 			usage();
 	} else 
@@ -306,6 +312,21 @@ bozo!
 #endif
 		exit(0);
 	}
+	if (oflag) {
+		long ncpus = -1;
+
+#if defined(_SC_NPROCESSORS_ONLN)
+		ncpus = sysconf(_SC_NPROCESSORS_ONLN);
+#elif defined(_SC_NPROC_ONLN)
+		ncpus = sysconf(_SC_NPROC_ONLN);
+#endif
+		if (ncpus == -1)
+			ncpus = 1;
+
+		printf("%ld\n", ncpus);
+
+		exit(0);
+	}
 
 	fprintf(stderr, "feature: dunno what you're after.\n");
 	return(1);
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH V3] xfstests: src/feature.c: print a number of online CPUs

[Stanislav Kholmanskikh]

On 10/29/2013 02:03 PM, Stanislav Kholmanskikh wrote:
> For this purpose we use sysconf() as it is the
> preferred platform neutral interface for getting this
> sort of information.
>
> Based on Dave Chinner proposal.
>
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
Hi!

Please, review.

Thanks.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH V3] xfstests: src/feature.c: print a number of online CPUs

[Rich Johnston]

Changes look good.

Reviewed-by: Rich Johnston <rjohnston@sgi.com>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH V3] xfstests: src/feature.c: print a number of online CPUs

[Rich Johnston]

This has been committed.

Thanks
--Rich

commit 2dcf4a56d118a8198424a1c30163f04fc04ddf32
Author: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
Date:   Tue Oct 29 10:03:59 2013 +0000

     xfstests: src/feature.c: print a number of online CPUs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH] xfstests: generic/273: use src/feature -o

[Stanislav Kholmanskikh]

Due to partially committed series (fd080d64b6e9677cb9d0a15dc7e308b6ca0e273f)
generic/273 test uses '_no_of_online_cpus' function which is not defined.

Now it's safe to switch it to 'src/feature -o'.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 tests/generic/273 |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tests/generic/273 b/tests/generic/273
index c028438..ab04dfa 100755
--- a/tests/generic/273
+++ b/tests/generic/273
@@ -47,7 +47,7 @@ count=2
 
 _threads_set()
 {
-	_cpu_num=`_no_of_online_cpus`
+	_cpu_num=`$here/src/feature -o`
 	threads=$(($_cpu_num * 50))
 	if [ $threads -gt 200 ]
 	then
-- 
1.7.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfstests: generic/273: use src/feature -o

[Jeff Liu]

On 11/11, 2013 17:34 PM, Stanislav Kholmanskikh wrote:
> Due to partially committed series (fd080d64b6e9677cb9d0a15dc7e308b6ca0e273f)
> generic/273 test uses '_no_of_online_cpus' function which is not defined.
> 
> Now it's safe to switch it to 'src/feature -o'.
> 
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
>  tests/generic/273 |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/tests/generic/273 b/tests/generic/273
> index c028438..ab04dfa 100755
> --- a/tests/generic/273
> +++ b/tests/generic/273
> @@ -47,7 +47,7 @@ count=2
>  
>  _threads_set()
>  {
> -	_cpu_num=`_no_of_online_cpus`
> +	_cpu_num=`$here/src/feature -o`
>  	threads=$(($_cpu_num * 50))
>  	if [ $threads -gt 200 ]
>  	then
Looks good to me.
Reviewed-by: Jie Liu <jeff.liu@oracle.com>

Thanks,
-Jeff

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/2] xfstests: generic/273: do not use /proc/cpuinfo

[Rich Johnston]

On 10/23/2013 08:09 PM, Dave Chinner wrote:
> On Wed, Oct 23, 2013 at 07:14:40PM -0500, Rich Johnston wrote:
>> This has been committed.
>
> Without commiting the other patch that implements
> _no_of_online_cpus....
>
>> Thanks
>> --Rich
>>
>> commit fd080d64b6e9677cb9d0a15dc7e308b6ca0e273f
>> Author: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
>> Date:   Wed Oct 23 11:58:44 2013 +0000
>>
>>      xfstests: generic/273: do not use /proc/cpuinfo
>
> generic/273 22s ... - output mismatch (see /home/dave/src/xfstests-dev/results//generic/273.out.bad)
>      --- tests/generic/273.out   2013-03-28 07:53:08.000000000 +1100
>      +++ /home/dave/src/xfstests-dev/results//generic/273.out.bad        2013-10-24 12:06:27.000000000 +1100
>      @@ -2,3 +2,5 @@
>       ------------------------------
>       start the workload
>       ------------------------------
>      +./tests/generic/273: line 50: _no_of_online_cpus: command not found
>      +./tests/generic/273: line 51: * 50: syntax error: operand expected (error token is "* 50")
>       ...
>       (Run 'diff -u tests/generic/273.out /home/dave/src/xfstests-dev/results//generic/273.out.bad' to see the entire diff)
> Ran: generic/273
> Failures: generic/273
> Failed 1 of 1 tests
>
> Rich, can you try to commit patch series as a whole, not piecemeal
> while parts of the patch series are still being discussed and
> reviewed?
>
> Cheers,
>
> Dave.
>
My apologies, I will look for series thread on the list as well as 
patchworks. (http://patchwork.xfs.org/project/XFS/list/). I did ask you 
about submitting some of the series you submitted  for review.
I did verify the the new function worked but I should have also run the
test suit also.


Two things, series patches should be:
	1. Resubmitted together with reviewed by's carried forward.
	2. If only one patch is being modified the patch caries the
	   series number along with the revision in the email subject.

#1
With the exception of your last submission (db series 37/37) Dave you
have always done this. I did carried the Reviewed-by: forward in that
case.

#2
You have always done this i.e
	[PATCH 15/37,V3] libxfs: fix root inode handling inconsistencies

In this case only the version was carried forward.
original
	 [PATCH 1/2] xfstests: implemented _no_of_online_cpus() function

The series was dropped from the subject.
  	[PATCH V3] xfstests: implemented _no_of_online_cpus() function

Thanks
--Rich
	

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfstests: generic/273: use src/feature -o

[Rich Johnston]

Sorry for the mis-communication in getting this series applied.
This has been committed.

Thanks
--Rich

commit bfdd1e72b358c6eba9584988d599aa3857461eb4
Author: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
Date:   Tue Oct 29 09:25:24 2013 +0000

     xfstests: added -P option to $DF_PROG

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfstests: generic/273: use src/feature -o

[Rich Johnston]

On 11/11/2013 10:05 AM, Rich Johnston wrote:
> Sorry for the mis-communication in getting this series applied.
> This has been committed.
>
> Thanks
> --Rich
>
Bad day this is the commit

commit efbdf561bd9a7124b3a35601b2ca036ff2f88dce
Author: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
Date:   Mon Nov 11 09:34:54 2013 +0000

     xfstests: generic/273: use src/feature -o

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs