lkml.org 
[lkml]   [2019]   [Mar]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3] cfi: fix deadloop in cfi_cmdset_0002.c do_write_buffer
Hi Ikegami,

On Fri, 1 Mar 2019 23:51:16 +0900
"Tokunori Ikegami" <ikegami_to@yahoo.co.jp> wrote:

> > Except this version no longer does what Vignesh suggested. See how you
> > no longer test if chip_good() is true if time_after() returns true. So,
> > imagine the thread entering this function is preempted just after the
> > first chip_good() test, and resumed a few ms later. time_after() will
> > return true, but chip_good() might also return true, and you ignore it.
>
> I think that the following 3 versions will be worked for time_after() as a same result and follow the Vignesh-san suggestion.

Let me show you how they are different:

>
> 1. Original Vignesh-san suggestion
>
> if (chip_good(map, adr, datum)) {
> xip_enable(map, chip, adr);
> goto op_done;
> }

--> thread preempted here
==> chip_good() test becomes valid here
--> thread resumed here, but timeout has expired

>
> if (time_after(jiffies, timeo)) {

you enter this branch

> /* Test chip_good() if TRUE incorrectly again so write failure by time_after() can be avoided. */
> if (chip_good(map, adr, datum)) {

chip_good() returns true

> xip_enable(map, chip, adr);
> goto op_done;
> }
> break;
> }
>
> 2. Liujian-san v3 patch
>
> /* Test chip_good() if FALSE correctly so write failure by time_after() can be avoided. */

--> thread preempted here
==> chip_good() test becomes valid here
--> thread resumed here, but timeout has expired

> if (time_after(jiffies, timeo) && !chip_good(map, adr))

You do not enter this branch because the chip_good() test is done once
more in case of timeout.

> break;
>
> if (chip_good(map, adr, datum)) {
> xip_enable(map, chip, adr);
> goto op_done;
> }
>
> 3. My idea
>
> /* Save current jiffies value before chip_good() to avoid write failure by time_after() without testing chip_good() again. */
> unsigned long now = jiffies;
>
> if (chip_good(map, adr, datum)) {
> xip_enable(map, chip, adr);
> goto op_done;
> }
>

--> thread preempted here
==> chip_good() test becomes valid here
--> thread resumed here, but timeout has expired

> if (time_after(now, timeo))

You do enter this branch, and erroneously report a failure.

> break;
>

See now why your version is not correct?

Regards,

Boris

\
 
 \ /
  Last update: 2019-03-01 17:08    [W:0.226 / U:0.340 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site