[dpdk-dev] [PATCH] eal/ppc64: revert implement arch-specific TSC freq query

[dpdk-dev] [PATCH] eal/ppc64: revert implement arch-specific TSC freq query

[Gowrishankar]

From: Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>

This reverts commit 15692396fd68932b6a81f00f12d4b0da12baa7d3
(eal/ppc64: implement arch-specific TSC freq query). We intended
to derive pkt/sec estimation with cpu clock frequency. As timebase
register serves the timer purpose, we need to stick with it for
calculating pkt/sec, hence reverting the change.

Fixes: 15692396fd (eal/ppc64: implement arch-specific TSC freq query)

Signed-off-by: Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>
---
 lib/librte_eal/common/arch/ppc_64/rte_cycles.c | 47 +-------------------------
 1 file changed, 1 insertion(+), 46 deletions(-)

diff --git a/lib/librte_eal/common/arch/ppc_64/rte_cycles.c b/lib/librte_eal/common/arch/ppc_64/rte_cycles.c
index 69a9f74..851fd02 100644
--- a/lib/librte_eal/common/arch/ppc_64/rte_cycles.c
+++ b/lib/librte_eal/common/arch/ppc_64/rte_cycles.c
@@ -1,52 +1,7 @@
-/*
- *   BSD LICENSE
- *
- *   Copyright (C) IBM Corporation 2014.
- *
- *   Redistribution and use in source and binary forms, with or without
- *   modification, are permitted provided that the following conditions
- *   are met:
- *
- *     * Redistributions of source code must retain the above copyright
- *       notice, this list of conditions and the following disclaimer.
- *     * Redistributions in binary form must reproduce the above copyright
- *       notice, this list of conditions and the following disclaimer in
- *       the documentation and/or other materials provided with the
- *       distribution.
- *     * Neither the name of IBM Corporation nor the names of its
- *       contributors may be used to endorse or promote products derived
- *       from this software without specific prior written permission.
- *
- *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#include <rte_lcore.h>
-#include <rte_log.h>
-#include "eal_filesystem.h"
 #include "eal_private.h"
 
-static const char sys_cpu_dir[] = "/sys/devices/system/cpu";
-
 uint64_t
 get_tsc_freq_arch(void)
 {
-	unsigned long cpu_hz;
-	char path[PATH_MAX];
-
-	snprintf(path, sizeof(path), "%s/cpu%d/cpufreq/cpuinfo_cur_freq",
-			sys_cpu_dir, rte_get_master_lcore());
-	if (eal_parse_sysfs_value(path, &cpu_hz) < 0)
-		RTE_LOG(WARNING, EAL, "Unable to parse %s\n", path);
-
-	return cpu_hz*1000;
+	return 0;
 }
-- 
1.9.1

Re: [dpdk-dev] [PATCH] eal/ppc64: revert implement arch-specific TSC freq query

[Thomas Monjalon]

30/01/2018 09:59, Gowrishankar:
> From: Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>
> 
> This reverts commit 15692396fd68932b6a81f00f12d4b0da12baa7d3
> (eal/ppc64: implement arch-specific TSC freq query). We intended
> to derive pkt/sec estimation with cpu clock frequency. As timebase
> register serves the timer purpose, we need to stick with it for
> calculating pkt/sec, hence reverting the change.
> 
> Fixes: 15692396fd (eal/ppc64: implement arch-specific TSC freq query)

It is not clear what it is fixing.

Chao, please review.

Re: [dpdk-dev] [PATCH] eal/ppc64: revert implement arch-specific TSC freq query

[Chao Zhu]

> -----Original Message-----
> From: Gowrishankar [mailto:gowrishankar.m@linux.vnet.ibm.com]
> Sent: 2018年1月30日 16:59
> To: dev@dpdk.org
> Cc: Chao Zhu <chaozhu@linux.vnet.ibm.com>; thomas@monjalon.net;
> Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>
> Subject: [PATCH] eal/ppc64: revert implement arch-specific TSC freq query
> 
> From: Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>
> 
> This reverts commit 15692396fd68932b6a81f00f12d4b0da12baa7d3
> (eal/ppc64: implement arch-specific TSC freq query). We intended to derive
> pkt/sec estimation with cpu clock frequency. As timebase register serves
the
> timer purpose, we need to stick with it for calculating pkt/sec, hence
reverting
> the change.
> 
> Fixes: 15692396fd (eal/ppc64: implement arch-specific TSC freq query)
> 
> Signed-off-by: Gowrishankar Muthukrishnan
> <gowrishankar.m@linux.vnet.ibm.com>
> ---
>  lib/librte_eal/common/arch/ppc_64/rte_cycles.c | 47
+-------------------------
>  1 file changed, 1 insertion(+), 46 deletions(-)
> 
> diff --git a/lib/librte_eal/common/arch/ppc_64/rte_cycles.c
> b/lib/librte_eal/common/arch/ppc_64/rte_cycles.c
> index 69a9f74..851fd02 100644
> --- a/lib/librte_eal/common/arch/ppc_64/rte_cycles.c
> +++ b/lib/librte_eal/common/arch/ppc_64/rte_cycles.c
> @@ -1,52 +1,7 @@
> -/*
> - *   BSD LICENSE
> - *
> - *   Copyright (C) IBM Corporation 2014.
> - *
> - *   Redistribution and use in source and binary forms, with or without
> - *   modification, are permitted provided that the following conditions
> - *   are met:
> - *
> - *     * Redistributions of source code must retain the above copyright
> - *       notice, this list of conditions and the following disclaimer.
> - *     * Redistributions in binary form must reproduce the above
copyright
> - *       notice, this list of conditions and the following disclaimer in
> - *       the documentation and/or other materials provided with the
> - *       distribution.
> - *     * Neither the name of IBM Corporation nor the names of its
> - *       contributors may be used to endorse or promote products derived
> - *       from this software without specific prior written permission.
> - *
> - *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS
> - *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT
> NOT
> - *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
> FITNESS FOR
> - *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> COPYRIGHT
> - *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> INCIDENTAL,
> - *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> NOT
> - *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
> OF USE,
> - *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
> AND ON ANY
> - *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> TORT
> - *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
> THE USE
> - *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> DAMAGE.
> - */
> -
> -#include <rte_lcore.h>
> -#include <rte_log.h>
> -#include "eal_filesystem.h"
>  #include "eal_private.h"
> 
> -static const char sys_cpu_dir[] = "/sys/devices/system/cpu";
> -
>  uint64_t
>  get_tsc_freq_arch(void)
>  {
> -	unsigned long cpu_hz;
> -	char path[PATH_MAX];
> -
> -	snprintf(path, sizeof(path), "%s/cpu%d/cpufreq/cpuinfo_cur_freq",
> -			sys_cpu_dir, rte_get_master_lcore());
> -	if (eal_parse_sysfs_value(path, &cpu_hz) < 0)
> -		RTE_LOG(WARNING, EAL, "Unable to parse %s\n", path);
> -
> -	return cpu_hz*1000;
> +	return 0;
>  }
> --
> 1.9.1
Acked-by: Chao Zhu <chaozhu@linux.vnet.ibm.com>

Re: [dpdk-dev] [PATCH] eal/ppc64: revert implement arch-specific TSC freq query

[Chao Zhu]

The previous patch has some misunderstanding of the the TSC frequency
counting.
I support this.

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: 2018年2月1日 8:30
> To: Gowrishankar <gowrishankar.m@linux.vnet.ibm.com>; Chao Zhu
> <chaozhu@linux.vnet.ibm.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] eal/ppc64: revert implement arch-specific
TSC
> freq query
> 
> 30/01/2018 09:59, Gowrishankar:
> > From: Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>
> >
> > This reverts commit 15692396fd68932b6a81f00f12d4b0da12baa7d3
> > (eal/ppc64: implement arch-specific TSC freq query). We intended to
> > derive pkt/sec estimation with cpu clock frequency. As timebase
> > register serves the timer purpose, we need to stick with it for
> > calculating pkt/sec, hence reverting the change.
> >
> > Fixes: 15692396fd (eal/ppc64: implement arch-specific TSC freq query)
> 
> It is not clear what it is fixing.
> 
> Chao, please review.

Re: [dpdk-dev] [PATCH] eal/ppc64: revert implement arch-specific TSC freq query

[Thomas Monjalon]

07/02/2018 08:05, Chao Zhu:
> The previous patch has some misunderstanding of the the TSC frequency
> counting.
> I support this.

Chao, it is very late in the release cycle.
Do you want to take the risk of applying this patch in 18.02-rc4?
If you discover a bug in this patch next week, it will be too late
for fixing it in 18.02.


> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > Sent: 2018年2月1日 8:30
> > To: Gowrishankar <gowrishankar.m@linux.vnet.ibm.com>; Chao Zhu
> > <chaozhu@linux.vnet.ibm.com>
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] eal/ppc64: revert implement arch-specific
> TSC
> > freq query
> > 
> > 30/01/2018 09:59, Gowrishankar:
> > > From: Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>
> > >
> > > This reverts commit 15692396fd68932b6a81f00f12d4b0da12baa7d3
> > > (eal/ppc64: implement arch-specific TSC freq query). We intended to
> > > derive pkt/sec estimation with cpu clock frequency. As timebase
> > > register serves the timer purpose, we need to stick with it for
> > > calculating pkt/sec, hence reverting the change.
> > >
> > > Fixes: 15692396fd (eal/ppc64: implement arch-specific TSC freq query)
> > 
> > It is not clear what it is fixing.
> > 
> > Chao, please review.
> 
> 

Re: [dpdk-dev] [PATCH] eal/ppc64: revert implement arch-specific TSC freq query

[Chao Zhu]

Thomas,

I think it's OK to apply this patch now. It's been verified before and it's important for the correctness of the cycle counting on POWER. 
Thank you!

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: 2018年2月7日 15:26
> To: Chao Zhu <chaozhu@linux.vnet.ibm.com>
> Cc: 'Gowrishankar' <gowrishankar.m@linux.vnet.ibm.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] eal/ppc64: revert implement arch-specific TSC
> freq query
> 
> 07/02/2018 08:05, Chao Zhu:
> > The previous patch has some misunderstanding of the the TSC frequency
> > counting.
> > I support this.
> 
> Chao, it is very late in the release cycle.
> Do you want to take the risk of applying this patch in 18.02-rc4?
> If you discover a bug in this patch next week, it will be too late for fixing it in
> 18.02.
> 
> 
> > > -----Original Message-----
> > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > Sent: 2018年2月1日 8:30
> > > To: Gowrishankar <gowrishankar.m@linux.vnet.ibm.com>; Chao Zhu
> > > <chaozhu@linux.vnet.ibm.com>
> > > Cc: dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH] eal/ppc64: revert implement
> > > arch-specific
> > TSC
> > > freq query
> > >
> > > 30/01/2018 09:59, Gowrishankar:
> > > > From: Gowrishankar Muthukrishnan
> > > > <gowrishankar.m@linux.vnet.ibm.com>
> > > >
> > > > This reverts commit 15692396fd68932b6a81f00f12d4b0da12baa7d3
> > > > (eal/ppc64: implement arch-specific TSC freq query). We intended
> > > > to derive pkt/sec estimation with cpu clock frequency. As timebase
> > > > register serves the timer purpose, we need to stick with it for
> > > > calculating pkt/sec, hence reverting the change.
> > > >
> > > > Fixes: 15692396fd (eal/ppc64: implement arch-specific TSC freq
> > > > query)
> > >
> > > It is not clear what it is fixing.
> > >
> > > Chao, please review.
> >
> >
> 
> 

Re: [dpdk-dev] [PATCH] eal/ppc64: revert implement arch-specific TSC freq query

[Thomas Monjalon]

> > This reverts commit 15692396fd68932b6a81f00f12d4b0da12baa7d3
> > (eal/ppc64: implement arch-specific TSC freq query). We intended to derive
> > pkt/sec estimation with cpu clock frequency. As timebase register serves
> the
> > timer purpose, we need to stick with it for calculating pkt/sec, hence
> reverting
> > the change.
> > 
> > Fixes: 15692396fd (eal/ppc64: implement arch-specific TSC freq query)
> > 
> > Signed-off-by: Gowrishankar Muthukrishnan
> > <gowrishankar.m@linux.vnet.ibm.com>
> > ---
> Acked-by: Chao Zhu <chaozhu@linux.vnet.ibm.com>

Applied, thanks