* [PATCH v2] drivers:soc:fsl:qbman:qman.c: Sleep instead of stuck hacking jiffies.
@ 2017-05-04 4:58 ` Karim Eshapa
0 siblings, 0 replies; 26+ messages in thread
From: Karim Eshapa @ 2017-05-04 4:58 UTC (permalink / raw)
To: linux-arm-kernel
Avoid stuck and hacking jiffies for a long time and using msleep()
for certatin numeber of cylces without the factor of safety
but using the the long delay considering the factor of safety
with the while loop such that after msleep for a short period
of time we check the msg if it's OK, breaking the big loop delay.
Signed-off-by: Karim Eshapa <karim.eshapa@gmail.com>
---
drivers/soc/fsl/qbman/qman.c | 47 ++++++++++++++++++++++----------------------
1 file changed, 24 insertions(+), 23 deletions(-)
diff --git a/drivers/soc/fsl/qbman/qman.c b/drivers/soc/fsl/qbman/qman.c
index 6f509f6..4f99298 100644
--- a/drivers/soc/fsl/qbman/qman.c
+++ b/drivers/soc/fsl/qbman/qman.c
@@ -1067,32 +1067,33 @@ static irqreturn_t portal_isr(int irq, void *ptr)
static int drain_mr_fqrni(struct qm_portal *p)
{
const union qm_mr_entry *msg;
+ unsigned long stop;
+ unsigned int timeout = jiffies_to_msecs(1000);
loop:
msg = qm_mr_current(p);
- if (!msg) {
- /*
- * if MR was full and h/w had other FQRNI entries to produce, we
- * need to allow it time to produce those entries once the
- * existing entries are consumed. A worst-case situation
- * (fully-loaded system) means h/w sequencers may have to do 3-4
- * other things before servicing the portal's MR pump, each of
- * which (if slow) may take ~50 qman cycles (which is ~200
- * processor cycles). So rounding up and then multiplying this
- * worst-case estimate by a factor of 10, just to be
- * ultra-paranoid, goes as high as 10,000 cycles. NB, we consume
- * one entry at a time, so h/w has an opportunity to produce new
- * entries well before the ring has been fully consumed, so
- * we're being *really* paranoid here.
- */
- u64 now, then = jiffies;
-
- do {
- now = jiffies;
- } while ((then + 10000) > now);
+ stop = jiffies + 10000;
+ /*
+ * if MR was full and h/w had other FQRNI entries to produce, we
+ * need to allow it time to produce those entries once the
+ * existing entries are consumed. A worst-case situation
+ * (fully-loaded system) means h/w sequencers may have to do 3-4
+ * other things before servicing the portal's MR pump, each of
+ * which (if slow) may take ~50 qman cycles (which is ~200
+ * processor cycles). So rounding up and then multiplying this
+ * worst-case estimate by a factor of 10, just to be
+ * ultra-paranoid, goes as high as 10,000 cycles. NB, we consume
+ * one entry at a time, so h/w has an opportunity to produce new
+ * entries well before the ring has been fully consumed, so
+ * we're being *really* paranoid here.
+ */
+ do {
+ if (msg)
+ break;
+ msleep(timeout);
msg = qm_mr_current(p);
- if (!msg)
- return 0;
- }
+ } while (time_before(jiffies, stop));
+ if (!msg)
+ return 0;
if ((msg->verb & QM_MR_VERB_TYPE_MASK) != QM_MR_VERB_FQRNI) {
/* We aren't draining anything but FQRNIs */
pr_err("Found verb 0x%x in MR\n", msg->verb);
--
2.7.4
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2] drivers:soc:fsl:qbman:qman.c: Sleep instead of stuck hacking jiffies.
2017-05-04 4:58 ` Karim Eshapa
@ 2017-05-04 21:07 ` Scott Wood
-1 siblings, 0 replies; 26+ messages in thread
From: Scott Wood @ 2017-05-04 21:07 UTC (permalink / raw)
To: Karim Eshapa
Cc: claudiu.manoil, roy.pledge, colin.king, linuxppc-dev,
linux-arm-kernel, linux-kernel
On Thu, 2017-05-04 at 06:58 +0200, Karim Eshapa wrote:
> + stop = jiffies + 10000;
> + /*
> + * if MR was full and h/w had other FQRNI entries to produce, we
> + * need to allow it time to produce those entries once the
> + * existing entries are consumed. A worst-case situation
> + * (fully-loaded system) means h/w sequencers may have to do 3-4
> + * other things before servicing the portal's MR pump, each of
> + * which (if slow) may take ~50 qman cycles (which is ~200
> + * processor cycles). So rounding up and then multiplying this
> + * worst-case estimate by a factor of 10, just to be
> + * ultra-paranoid, goes as high as 10,000 cycles. NB, we consume
> + * one entry at a time, so h/w has an opportunity to produce new
> + * entries well before the ring has been fully consumed, so
> + * we're being *really* paranoid here.
> + */
OK, upon reading this more closely it seems the intent was to delay for 10,000
*processor cycles* and somehow that got turned into 10,000 jiffies (which is
40 seconds at the default Hz!). We could just replace this whole thing with
msleep(1) and still be far more paranoid than was originally intended.
Claudiu and Roy, any comments?
-Scott
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2] drivers:soc:fsl:qbman:qman.c: Sleep instead of stuck hacking jiffies.
@ 2017-05-04 21:07 ` Scott Wood
0 siblings, 0 replies; 26+ messages in thread
From: Scott Wood @ 2017-05-04 21:07 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 2017-05-04 at 06:58 +0200, Karim Eshapa wrote:
> + stop = jiffies + 10000;
> + /*
> + ?* if MR was full and h/w had other FQRNI entries to produce, we
> + ?* need to allow it time to produce those entries once the
> + ?* existing entries are consumed. A worst-case situation
> + ?* (fully-loaded system) means h/w sequencers may have to do 3-4
> + ?* other things before servicing the portal's MR pump, each of
> + ?* which (if slow) may take ~50 qman cycles (which is ~200
> + ?* processor cycles). So rounding up and then multiplying this
> + ?* worst-case estimate by a factor of 10, just to be
> + ?* ultra-paranoid, goes as high as 10,000 cycles. NB, we consume
> + ?* one entry at a time, so h/w has an opportunity to produce new
> + ?* entries well before the ring has been fully consumed, so
> + ?* we're being *really* paranoid here.
> + ?*/
OK, upon reading this more closely it seems the intent was to delay for 10,000
*processor cycles* and somehow that got turned into 10,000 jiffies (which is
40 seconds at the default Hz!). We could just replace this whole thing with
msleep(1) and still be far more paranoid than was originally intended.
Claudiu and Roy, any comments?
-Scott
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2] drivers:soc:fsl:qbman:qman.c: Sleep instead of stuck hacking jiffies.
2017-05-04 21:07 ` Scott Wood
(?)
@ 2017-05-04 23:30 ` Roy Pledge
-1 siblings, 0 replies; 26+ messages in thread
From: Roy Pledge @ 2017-05-04 23:30 UTC (permalink / raw)
To: Scott Wood, Karim Eshapa
Cc: Claudiu Manoil, colin.king@canonical.com,
linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
On 5/4/2017 5:07 PM, Scott Wood wrote:=0A=
> On Thu, 2017-05-04 at 06:58 +0200, Karim Eshapa wrote:=0A=
>> + stop =3D jiffies + 10000;=0A=
>> + /*=0A=
>> + * if MR was full and h/w had other FQRNI entries to produce, we=0A=
>> + * need to allow it time to produce those entries once the=0A=
>> + * existing entries are consumed. A worst-case situation=0A=
>> + * (fully-loaded system) means h/w sequencers may have to do 3-4=0A=
>> + * other things before servicing the portal's MR pump, each of=0A=
>> + * which (if slow) may take ~50 qman cycles (which is ~200=0A=
>> + * processor cycles). So rounding up and then multiplying this=0A=
>> + * worst-case estimate by a factor of 10, just to be=0A=
>> + * ultra-paranoid, goes as high as 10,000 cycles. NB, we consume=0A=
>> + * one entry at a time, so h/w has an opportunity to produce new=0A=
>> + * entries well before the ring has been fully consumed, so=0A=
>> + * we're being *really* paranoid here.=0A=
>> + */=0A=
> OK, upon reading this more closely it seems the intent was to delay for 1=
0,000=0A=
> *processor cycles* and somehow that got turned into 10,000 jiffies (which=
is=0A=
> 40 seconds at the default Hz!). We could just replace this whole thing w=
ith=0A=
> msleep(1) and still be far more paranoid than was originally intended.=0A=
>=0A=
> Claudiu and Roy, any comments?=0A=
Yes the timing here is certainly off, the code changed a few times since=0A=
the comment was originally written.=0A=
An msleep(1) seems reasonable here to me.=0A=
=0A=
Roy=0A=
>=0A=
> -Scott=0A=
>=0A=
>=0A=
=0A=
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2] drivers:soc:fsl:qbman:qman.c: Sleep instead of stuck hacking jiffies.
@ 2017-05-04 23:30 ` Roy Pledge
0 siblings, 0 replies; 26+ messages in thread
From: Roy Pledge @ 2017-05-04 23:30 UTC (permalink / raw)
To: Scott Wood, Karim Eshapa
Cc: Claudiu Manoil, colin.king@canonical.com,
linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
On 5/4/2017 5:07 PM, Scott Wood wrote:
> On Thu, 2017-05-04 at 06:58 +0200, Karim Eshapa wrote:
>> + stop = jiffies + 10000;
>> + /*
>> + * if MR was full and h/w had other FQRNI entries to produce, we
>> + * need to allow it time to produce those entries once the
>> + * existing entries are consumed. A worst-case situation
>> + * (fully-loaded system) means h/w sequencers may have to do 3-4
>> + * other things before servicing the portal's MR pump, each of
>> + * which (if slow) may take ~50 qman cycles (which is ~200
>> + * processor cycles). So rounding up and then multiplying this
>> + * worst-case estimate by a factor of 10, just to be
>> + * ultra-paranoid, goes as high as 10,000 cycles. NB, we consume
>> + * one entry at a time, so h/w has an opportunity to produce new
>> + * entries well before the ring has been fully consumed, so
>> + * we're being *really* paranoid here.
>> + */
> OK, upon reading this more closely it seems the intent was to delay for 10,000
> *processor cycles* and somehow that got turned into 10,000 jiffies (which is
> 40 seconds at the default Hz!). We could just replace this whole thing with
> msleep(1) and still be far more paranoid than was originally intended.
>
> Claudiu and Roy, any comments?
Yes the timing here is certainly off, the code changed a few times since
the comment was originally written.
An msleep(1) seems reasonable here to me.
Roy
>
> -Scott
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2] drivers:soc:fsl:qbman:qman.c: Sleep instead of stuck hacking jiffies.
@ 2017-05-04 23:30 ` Roy Pledge
0 siblings, 0 replies; 26+ messages in thread
From: Roy Pledge @ 2017-05-04 23:30 UTC (permalink / raw)
To: linux-arm-kernel
On 5/4/2017 5:07 PM, Scott Wood wrote:
> On Thu, 2017-05-04 at 06:58 +0200, Karim Eshapa wrote:
>> + stop = jiffies + 10000;
>> + /*
>> + * if MR was full and h/w had other FQRNI entries to produce, we
>> + * need to allow it time to produce those entries once the
>> + * existing entries are consumed. A worst-case situation
>> + * (fully-loaded system) means h/w sequencers may have to do 3-4
>> + * other things before servicing the portal's MR pump, each of
>> + * which (if slow) may take ~50 qman cycles (which is ~200
>> + * processor cycles). So rounding up and then multiplying this
>> + * worst-case estimate by a factor of 10, just to be
>> + * ultra-paranoid, goes as high as 10,000 cycles. NB, we consume
>> + * one entry at a time, so h/w has an opportunity to produce new
>> + * entries well before the ring has been fully consumed, so
>> + * we're being *really* paranoid here.
>> + */
> OK, upon reading this more closely it seems the intent was to delay for 10,000
> *processor cycles* and somehow that got turned into 10,000 jiffies (which is
> 40 seconds at the default Hz!). We could just replace this whole thing with
> msleep(1) and still be far more paranoid than was originally intended.
>
> Claudiu and Roy, any comments?
Yes the timing here is certainly off, the code changed a few times since
the comment was originally written.
An msleep(1) seems reasonable here to me.
Roy
>
> -Scott
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH v3] drivers:soc:fsl:qbman:qman.c: Sleep instead of stuck hacking jiffies.
2017-05-04 4:58 ` Karim Eshapa
@ 2017-05-05 5:45 ` Karim Eshapa
-1 siblings, 0 replies; 26+ messages in thread
From: Karim Eshapa @ 2017-05-05 5:45 UTC (permalink / raw)
To: oss
Cc: claudiu.manoil, roy.pledge, colin.king, linuxppc-dev,
linux-arm-kernel, linux-kernel, Karim Eshapa
Use msleep() instead of stucking with
long delay will be more efficient.
Signed-off-by: Karim Eshapa <karim.eshapa@gmail.com>
---
drivers/soc/fsl/qbman/qman.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/soc/fsl/qbman/qman.c b/drivers/soc/fsl/qbman/qman.c
index 3d891db..18d391e 100644
--- a/drivers/soc/fsl/qbman/qman.c
+++ b/drivers/soc/fsl/qbman/qman.c
@@ -1084,11 +1084,7 @@ static int drain_mr_fqrni(struct qm_portal *p)
* entries well before the ring has been fully consumed, so
* we're being *really* paranoid here.
*/
- u64 now, then = jiffies;
-
- do {
- now = jiffies;
- } while ((then + 10000) > now);
+ msleep(1);
msg = qm_mr_current(p);
if (!msg)
return 0;
--
2.7.4
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v3] drivers:soc:fsl:qbman:qman.c: Sleep instead of stuck hacking jiffies.
@ 2017-05-05 5:45 ` Karim Eshapa
0 siblings, 0 replies; 26+ messages in thread
From: Karim Eshapa @ 2017-05-05 5:45 UTC (permalink / raw)
To: linux-arm-kernel
Use msleep() instead of stucking with
long delay will be more efficient.
Signed-off-by: Karim Eshapa <karim.eshapa@gmail.com>
---
drivers/soc/fsl/qbman/qman.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/soc/fsl/qbman/qman.c b/drivers/soc/fsl/qbman/qman.c
index 3d891db..18d391e 100644
--- a/drivers/soc/fsl/qbman/qman.c
+++ b/drivers/soc/fsl/qbman/qman.c
@@ -1084,11 +1084,7 @@ static int drain_mr_fqrni(struct qm_portal *p)
* entries well before the ring has been fully consumed, so
* we're being *really* paranoid here.
*/
- u64 now, then = jiffies;
-
- do {
- now = jiffies;
- } while ((then + 10000) > now);
+ msleep(1);
msg = qm_mr_current(p);
if (!msg)
return 0;
--
2.7.4
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [v3] drivers:soc:fsl:qbman:qman.c: Sleep instead of stuck hacking jiffies.
2017-05-05 5:45 ` Karim Eshapa
@ 2017-06-25 2:46 ` Scott Wood
-1 siblings, 0 replies; 26+ messages in thread
From: Scott Wood @ 2017-06-25 2:46 UTC (permalink / raw)
To: Karim Eshapa
Cc: roy.pledge, linux-kernel, claudiu.manoil, colin.king,
linuxppc-dev, linux-arm-kernel, Li Yang
On Fri, May 05, 2017 at 07:45:18AM +0200, Karim Eshapa wrote:
> Use msleep() instead of stucking with
> long delay will be more efficient.
>
> Signed-off-by: Karim Eshapa <karim.eshapa@gmail.com>
> ---
> drivers/soc/fsl/qbman/qman.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
Acked-by: Scott Wood <oss@buserror.net>
(though the subject line should be "soc/qman: ...")
Leo, are you going to send this patch (and other qman patches) via
arm-soc?
-Scott
^ permalink raw reply [flat|nested] 26+ messages in thread
* [v3] drivers:soc:fsl:qbman:qman.c: Sleep instead of stuck hacking jiffies.
@ 2017-06-25 2:46 ` Scott Wood
0 siblings, 0 replies; 26+ messages in thread
From: Scott Wood @ 2017-06-25 2:46 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, May 05, 2017 at 07:45:18AM +0200, Karim Eshapa wrote:
> Use msleep() instead of stucking with
> long delay will be more efficient.
>
> Signed-off-by: Karim Eshapa <karim.eshapa@gmail.com>
> ---
> drivers/soc/fsl/qbman/qman.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
Acked-by: Scott Wood <oss@buserror.net>
(though the subject line should be "soc/qman: ...")
Leo, are you going to send this patch (and other qman patches) via
arm-soc?
-Scott
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [v3] drivers:soc:fsl:qbman:qman.c: Sleep instead of stuck hacking jiffies.
2017-06-25 2:46 ` Scott Wood
(?)
@ 2017-06-27 16:38 ` Leo Li
-1 siblings, 0 replies; 26+ messages in thread
From: Leo Li @ 2017-06-27 16:38 UTC (permalink / raw)
To: Scott Wood, Karim Eshapa
Cc: Roy Pledge, linux-kernel@vger.kernel.org, Claudiu Manoil,
colin.king@canonical.com, linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
> -----Original Message-----
> From: Scott Wood [mailto:oss@buserror.net]
> Sent: Saturday, June 24, 2017 9:47 PM
> To: Karim Eshapa <karim.eshapa@gmail.com>
> Cc: Roy Pledge <roy.pledge@nxp.com>; linux-kernel@vger.kernel.org;
> Claudiu Manoil <claudiu.manoil@nxp.com>; colin.king@canonical.com;
> linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org; Leo =
Li
> <leoyang.li@nxp.com>
> Subject: Re: [v3] drivers:soc:fsl:qbman:qman.c: Sleep instead of stuck
> hacking jiffies.
>=20
> On Fri, May 05, 2017 at 07:45:18AM +0200, Karim Eshapa wrote:
> > Use msleep() instead of stucking with
> > long delay will be more efficient.
> >
> > Signed-off-by: Karim Eshapa <karim.eshapa@gmail.com>
> > ---
> > drivers/soc/fsl/qbman/qman.c | 6 +-----
> > 1 file changed, 1 insertion(+), 5 deletions(-)
>=20
> Acked-by: Scott Wood <oss@buserror.net>
>=20
> (though the subject line should be "soc/qman: ...")
>=20
> Leo, are you going to send this patch (and other qman patches) via arm-so=
c?
Yes. I can take it through the pull request for soc/fsl via arm-soc. As m=
entioned in the feedback from David in another email, probably we should up=
date the comment and commit message to mention how 10000 cycles becomes 1ms=
.
Regards,
Leo
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [v3] drivers:soc:fsl:qbman:qman.c: Sleep instead of stuck hacking jiffies.
@ 2017-06-27 16:38 ` Leo Li
0 siblings, 0 replies; 26+ messages in thread
From: Leo Li @ 2017-06-27 16:38 UTC (permalink / raw)
To: Scott Wood, Karim Eshapa
Cc: Roy Pledge, linux-kernel@vger.kernel.org, Claudiu Manoil,
colin.king@canonical.com, linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
> -----Original Message-----
> From: Scott Wood [mailto:oss@buserror.net]
> Sent: Saturday, June 24, 2017 9:47 PM
> To: Karim Eshapa <karim.eshapa@gmail.com>
> Cc: Roy Pledge <roy.pledge@nxp.com>; linux-kernel@vger.kernel.org;
> Claudiu Manoil <claudiu.manoil@nxp.com>; colin.king@canonical.com;
> linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org; Leo Li
> <leoyang.li@nxp.com>
> Subject: Re: [v3] drivers:soc:fsl:qbman:qman.c: Sleep instead of stuck
> hacking jiffies.
>
> On Fri, May 05, 2017 at 07:45:18AM +0200, Karim Eshapa wrote:
> > Use msleep() instead of stucking with
> > long delay will be more efficient.
> >
> > Signed-off-by: Karim Eshapa <karim.eshapa@gmail.com>
> > ---
> > drivers/soc/fsl/qbman/qman.c | 6 +-----
> > 1 file changed, 1 insertion(+), 5 deletions(-)
>
> Acked-by: Scott Wood <oss@buserror.net>
>
> (though the subject line should be "soc/qman: ...")
>
> Leo, are you going to send this patch (and other qman patches) via arm-soc?
Yes. I can take it through the pull request for soc/fsl via arm-soc. As mentioned in the feedback from David in another email, probably we should update the comment and commit message to mention how 10000 cycles becomes 1ms.
Regards,
Leo
^ permalink raw reply [flat|nested] 26+ messages in thread
* [v3] drivers:soc:fsl:qbman:qman.c: Sleep instead of stuck hacking jiffies.
@ 2017-06-27 16:38 ` Leo Li
0 siblings, 0 replies; 26+ messages in thread
From: Leo Li @ 2017-06-27 16:38 UTC (permalink / raw)
To: linux-arm-kernel
> -----Original Message-----
> From: Scott Wood [mailto:oss at buserror.net]
> Sent: Saturday, June 24, 2017 9:47 PM
> To: Karim Eshapa <karim.eshapa@gmail.com>
> Cc: Roy Pledge <roy.pledge@nxp.com>; linux-kernel at vger.kernel.org;
> Claudiu Manoil <claudiu.manoil@nxp.com>; colin.king at canonical.com;
> linuxppc-dev at lists.ozlabs.org; linux-arm-kernel at lists.infradead.org; Leo Li
> <leoyang.li@nxp.com>
> Subject: Re: [v3] drivers:soc:fsl:qbman:qman.c: Sleep instead of stuck
> hacking jiffies.
>
> On Fri, May 05, 2017 at 07:45:18AM +0200, Karim Eshapa wrote:
> > Use msleep() instead of stucking with
> > long delay will be more efficient.
> >
> > Signed-off-by: Karim Eshapa <karim.eshapa@gmail.com>
> > ---
> > drivers/soc/fsl/qbman/qman.c | 6 +-----
> > 1 file changed, 1 insertion(+), 5 deletions(-)
>
> Acked-by: Scott Wood <oss@buserror.net>
>
> (though the subject line should be "soc/qman: ...")
>
> Leo, are you going to send this patch (and other qman patches) via arm-soc?
Yes. I can take it through the pull request for soc/fsl via arm-soc. As mentioned in the feedback from David in another email, probably we should update the comment and commit message to mention how 10000 cycles becomes 1ms.
Regards,
Leo
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH v2] drivers:soc:fsl:qbman:qman.c: Sleep instead of stuck hacking jiffies.
2017-05-04 4:58 ` Karim Eshapa
@ 2017-05-05 6:01 ` Karim Eshapa
-1 siblings, 0 replies; 26+ messages in thread
From: Karim Eshapa @ 2017-05-05 6:01 UTC (permalink / raw)
To: oss
Cc: claudiu.manoil, roy.pledge, colin.king, linuxppc-dev,
linux-arm-kernel, linux-kernel, Karim Eshapa
>On 5/4/2017 5:07 PM, Scott Wood wrote:
>> On Thu, 2017-05-04 at 06:58 +0200, Karim Eshapa wrote:
>>> + stop = jiffies + 10000;
>>> + /*
>>> + * if MR was full and h/w had other FQRNI entries to produce, we
>>> + * need to allow it time to produce those entries once the
>>> + * existing entries are consumed. A worst-case situation
>>> + * (fully-loaded system) means h/w sequencers may have to do 3-4
>>> + * other things before servicing the portal's MR pump, each of
>>> + * which (if slow) may take ~50 qman cycles (which is ~200
>>> + * processor cycles). So rounding up and then multiplying this
>>> + * worst-case estimate by a factor of 10, just to be
>>> + * ultra-paranoid, goes as high as 10,000 cycles. NB, we consume
>>> + * one entry at a time, so h/w has an opportunity to produce new
>>> + * entries well before the ring has been fully consumed, so
>>> + * we're being *really* paranoid here.
>>> + */
>> OK, upon reading this more closely it seems the intent was to delay for 10,000
>> *processor cycles* and somehow that got turned into 10,000 jiffies (which is
>> 40 seconds at the default Hz!). We could just replace this whole thing with
>> msleep(1) and still be far more paranoid than was originally intended.
>>
>> Claudiu and Roy, any comments?
>Yes the timing here is certainly off, the code changed a few times since
>the comment was originally written.
>An msleep(1) seems reasonable here to me.
If the previous patch with msleep(1) is OK.
can I send a patch to slightly change the comments.
Thanks,
Karim
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2] drivers:soc:fsl:qbman:qman.c: Sleep instead of stuck hacking jiffies.
@ 2017-05-05 6:01 ` Karim Eshapa
0 siblings, 0 replies; 26+ messages in thread
From: Karim Eshapa @ 2017-05-05 6:01 UTC (permalink / raw)
To: linux-arm-kernel
>On 5/4/2017 5:07 PM, Scott Wood wrote:
>> On Thu, 2017-05-04 at 06:58 +0200, Karim Eshapa wrote:
>>> + stop = jiffies + 10000;
>>> + /*
>>> + * if MR was full and h/w had other FQRNI entries to produce, we
>>> + * need to allow it time to produce those entries once the
>>> + * existing entries are consumed. A worst-case situation
>>> + * (fully-loaded system) means h/w sequencers may have to do 3-4
>>> + * other things before servicing the portal's MR pump, each of
>>> + * which (if slow) may take ~50 qman cycles (which is ~200
>>> + * processor cycles). So rounding up and then multiplying this
>>> + * worst-case estimate by a factor of 10, just to be
>>> + * ultra-paranoid, goes as high as 10,000 cycles. NB, we consume
>>> + * one entry at a time, so h/w has an opportunity to produce new
>>> + * entries well before the ring has been fully consumed, so
>>> + * we're being *really* paranoid here.
>>> + */
>> OK, upon reading this more closely it seems the intent was to delay for 10,000
>> *processor cycles* and somehow that got turned into 10,000 jiffies (which is
>> 40 seconds at the default Hz!). We could just replace this whole thing with
>> msleep(1) and still be far more paranoid than was originally intended.
>>
>> Claudiu and Roy, any comments?
>Yes the timing here is certainly off, the code changed a few times since
>the comment was originally written.
>An msleep(1) seems reasonable here to me.
If the previous patch with msleep(1) is OK.
can I send a patch to slightly change the comments.
Thanks,
Karim
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2] drivers:soc:fsl:qbman:qman.c: Sleep instead of stuck hacking jiffies.
2017-05-05 6:01 ` Karim Eshapa
@ 2017-05-05 6:33 ` Scott Wood
-1 siblings, 0 replies; 26+ messages in thread
From: Scott Wood @ 2017-05-05 6:33 UTC (permalink / raw)
To: Karim Eshapa
Cc: claudiu.manoil, roy.pledge, colin.king, linuxppc-dev,
linux-arm-kernel, linux-kernel
On Fri, 2017-05-05 at 08:01 +0200, Karim Eshapa wrote:
> > On 5/4/2017 5:07 PM, Scott Wood wrote:
> > > On Thu, 2017-05-04 at 06:58 +0200, Karim Eshapa wrote:
> > > > + stop = jiffies + 10000;
> > > > + /*
> > > > + * if MR was full and h/w had other FQRNI entries to produce, we
> > > > + * need to allow it time to produce those entries once the
> > > > + * existing entries are consumed. A worst-case situation
> > > > + * (fully-loaded system) means h/w sequencers may have to do 3-4
> > > > + * other things before servicing the portal's MR pump, each of
> > > > + * which (if slow) may take ~50 qman cycles (which is ~200
> > > > + * processor cycles). So rounding up and then multiplying this
> > > > + * worst-case estimate by a factor of 10, just to be
> > > > + * ultra-paranoid, goes as high as 10,000 cycles. NB, we consume
> > > > + * one entry at a time, so h/w has an opportunity to produce new
> > > > + * entries well before the ring has been fully consumed, so
> > > > + * we're being *really* paranoid here.
> > > > + */
> > >
> > > OK, upon reading this more closely it seems the intent was to delay for
> > > 10,000
> > > *processor cycles* and somehow that got turned into 10,000 jiffies
> > > (which is
> > > 40 seconds at the default Hz!). We could just replace this whole thing
> > > with
> > > msleep(1) and still be far more paranoid than was originally intended.
> > >
> > > Claudiu and Roy, any comments?
> >
> > Yes the timing here is certainly off, the code changed a few times since
> > the comment was originally written.
> > An msleep(1) seems reasonable here to me.
>
> If the previous patch with msleep(1) is OK.
> can I send a patch to slightly change the comments.
Yes.
-Scott
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2] drivers:soc:fsl:qbman:qman.c: Sleep instead of stuck hacking jiffies.
@ 2017-05-05 6:33 ` Scott Wood
0 siblings, 0 replies; 26+ messages in thread
From: Scott Wood @ 2017-05-05 6:33 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 2017-05-05 at 08:01 +0200, Karim Eshapa wrote:
> > On 5/4/2017 5:07 PM, Scott Wood wrote:
> > > On Thu, 2017-05-04 at 06:58 +0200, Karim Eshapa wrote:
> > > > +????stop = jiffies + 10000;
> > > > +????/*
> > > > +?????* if MR was full and h/w had other FQRNI entries to produce, we
> > > > +?????* need to allow it time to produce those entries once the
> > > > +?????* existing entries are consumed. A worst-case situation
> > > > +?????* (fully-loaded system) means h/w sequencers may have to do 3-4
> > > > +?????* other things before servicing the portal's MR pump, each of
> > > > +?????* which (if slow) may take ~50 qman cycles (which is ~200
> > > > +?????* processor cycles). So rounding up and then multiplying this
> > > > +?????* worst-case estimate by a factor of 10, just to be
> > > > +?????* ultra-paranoid, goes as high as 10,000 cycles. NB, we consume
> > > > +?????* one entry at a time, so h/w has an opportunity to produce new
> > > > +?????* entries well before the ring has been fully consumed, so
> > > > +?????* we're being *really* paranoid here.
> > > > +?????*/
> > >
> > > OK, upon reading this more closely it seems the intent was to delay for
> > > 10,000
> > > *processor cycles* and somehow that got turned into 10,000 jiffies
> > > (which is
> > > 40 seconds at the default Hz!).??We could just replace this whole thing
> > > with
> > > msleep(1) and still be far more paranoid than was originally intended.
> > >
> > > Claudiu and Roy, any comments?
> >
> > Yes the timing here is certainly off, the code changed a few times since
> > the comment was originally written.
> > An msleep(1) seems reasonable here to me.
>
> If the previous patch with msleep(1) is OK.
> can I send a patch to slightly change the comments.
Yes.
-Scott
^ permalink raw reply [flat|nested] 26+ messages in thread