Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1674239 > unrolled thread

[PATCH] soc/qman: Sleep instead of stuck hacking jiffies.

Back to article view | Back to linux.kernel

Contents

  [PATCH] soc/qman: Sleep instead of stuck hacking jiffies. Karim Eshapa <karim.eshapa@gmail.com> - 2017-06-25 17:20 +0200
    RE: [PATCH] soc/qman: Sleep instead of stuck hacking jiffies. David Laight <David.Laight@ACULAB.COM> - 2017-06-26 18:00 +0200
      RE: [PATCH] soc/qman: Sleep instead of stuck hacking jiffies. Leo Li <leoyang.li@nxp.com> - 2017-06-27 18:50 +0200

#1674239 — [PATCH] soc/qman: Sleep instead of stuck hacking jiffies.

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

[toc] | [next] | [standalone]

#1674890

From: Karim Eshapa
> Sent: 25 June 2017 16:14
> Use msleep() instead of stucking with
> long delay will be more efficient.
...
> --- 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);
...
How is that in any way equivalent?
If HZ is 1000 the old code loops for 10 seconds.
If HZ is 250 (common for some distros) it loops for 40 seconds.

Clearly both are horrid, but it isn't at all clear that a 1ms sleep
is performing the same job.

My guess is that this code is never called, and broken if actually called.

	David

[toc] | [prev] | [next] | [standalone]

#1675998

> -----Original Message-----
> From: Linuxppc-dev [mailto:linuxppc-dev-
> bounces+leoli=freescale.com@lists.ozlabs.org] On Behalf Of David Laight
> Sent: Monday, June 26, 2017 10:55 AM
> To: 'Karim Eshapa' <karim.eshapa@gmail.com>; oss@buserror.net
> 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
> Subject: RE: [PATCH] soc/qman: Sleep instead of stuck hacking jiffies.
> 
> From: Karim Eshapa
> > Sent: 25 June 2017 16:14
> > Use msleep() instead of stucking with
> > long delay will be more efficient.
> ...
> > --- 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);
> ...
> How is that in any way equivalent?
> If HZ is 1000 the old code loops for 10 seconds.
> If HZ is 250 (common for some distros) it loops for 40 seconds.
> 
> Clearly both are horrid, but it isn't at all clear that a 1ms sleep is performing
> the same job.
> 
> My guess is that this code is never called, and broken if actually called.

It was indeed broken.  The intent was to wait for 10000 cycles but mistakenly coded as 10000 jiffies.  I think we choose 1ms as it is not too long and almost guarantees the 10000 cycles delay.

Regards,
Leo

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web