On Tue, Oct 30, 2018 at 11:25:11AM -0700, David Miller wrote: > From: Kurt Kanzenbach <[email protected]> > 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 <[email protected]> > ... > > 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.
That's true. The problem is different: If the current task gets preempted by a higher priority task between checking the condition and the timeout code, then a timeout might be falsely detected. Consider the following events: loop: check mdio condition ------------------------ task with real time priority may run for a long time ------------------------ check for timeout wait That's why I've added the recheck of the condition in the timeout case. > > 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. The timeout value is not the problem here. Thanks, Kurt
