LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: kurt@linutronix.de
Cc: anirudh@xilinx.com, John.Linn@xilinx.com,
	michal.simek@xilinx.com, radhey.shyam.pandey@xilinx.com,
	andrew@lunn.ch, yuehaibing@huawei.com, netdev@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] net: axienet: recheck condition after timeout in mdio_wait()
Date: Tue, 30 Oct 2018 11:25:11 -0700 (PDT)	[thread overview]
Message-ID: <20181030.112511.957438343014682098.davem@davemloft.net> (raw)
In-Reply-To: <20181030093139.10226-2-kurt@linutronix.de>

From: Kurt Kanzenbach <kurt@linutronix.de>
Date: Tue, 30 Oct 2018 10:31:38 +0100

> The function could report a false positive if it gets preempted between reading
> the XAE_MDIO_MCR_OFFSET register and checking for the timeout.  In such a case,
> the condition has to be rechecked to avoid false positives.
> 
> Therefore, check for expected condition even after the timeout occurred.
> 
> Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
 ...
>  		if (time_before_eq(end, jiffies)) {
> -			WARN_ON(1);
> -			return -ETIMEDOUT;
> +			val = axienet_ior(lp, XAE_MDIO_MCR_OFFSET);
> +			break;
>  		}
> +
>  		udelay(1);
>  	}
> -	return 0;
> +	if (val & XAE_MDIO_MCR_READY_MASK)
> +		return 0;
> +
> +	WARN_ON(1);
> +	return -ETIMEDOUT;

You are not fundamentally changing the situation at all.

The condtion could change right after your last read of
XAR_MDIO_MCR_OFFSET, which is the same thing that happens before your
modifications to this code.

It sounds more like the timeout is slightly too short, and that's the
real problem that causes whatever behavior you think you are fixing
here.

I'm not applying this.

  reply	other threads:[~2018-10-30 18:25 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-30  9:31 [PATCH 0/2] net: xlinx: mdio: recheck condition after timeout Kurt Kanzenbach
2018-10-30  9:31 ` [PATCH 1/2] net: axienet: recheck condition after timeout in mdio_wait() Kurt Kanzenbach
2018-10-30 18:25   ` David Miller [this message]
2018-10-31 13:06     ` Kurt Kanzenbach
2018-11-05  9:16     ` Sebastian Andrzej Siewior
2018-10-30  9:31 ` [PATCH 2/2] net: xilinx_emaclite: " Kurt Kanzenbach
2018-10-30 12:10   ` Andrew Lunn
2018-10-30 12:58     ` Radhey Shyam Pandey
2018-10-30 18:25   ` David Miller
2018-10-30 12:08 ` [PATCH 0/2] net: xlinx: mdio: recheck condition after timeout Andrew Lunn
2018-10-30 13:47   ` Kurt Kanzenbach

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181030.112511.957438343014682098.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=John.Linn@xilinx.com \
    --cc=andrew@lunn.ch \
    --cc=anirudh@xilinx.com \
    --cc=kurt@linutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michal.simek@xilinx.com \
    --cc=netdev@vger.kernel.org \
    --cc=radhey.shyam.pandey@xilinx.com \
    --cc=yuehaibing@huawei.com \
    --subject='Re: [PATCH 1/2] net: axienet: recheck condition after timeout in mdio_wait()' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).