linux-kernel - Re: [PATCH 06/11] mtd: rawnand: denali: use more precise timeout for NAND_OP_WAITRDT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 11 Feb 2019 10:26:39 +0900
From:   Masahiro Yamada <yamada.masahiro@...ionext.com>
To:     Miquel Raynal <miquel.raynal@...tlin.com>
Cc:     linux-mtd <linux-mtd@...ts.infradead.org>,
        Boris Brezillon <bbrezillon@...nel.org>,
        Brian Norris <computersforpeace@...il.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Marek Vasut <marek.vasut@...il.com>,
        Richard Weinberger <richard@....at>,
        David Woodhouse <dwmw2@...radead.org>
Subject: Re: [PATCH 06/11] mtd: rawnand: denali: use more precise timeout for NAND_OP_WAITRDT_INSTR

Hi Miquel,

On Sat, Feb 9, 2019 at 7:05 AM Miquel Raynal <miquel.raynal@...tlin.com> wrote:
>
> Hi Masahiro,
>
> Masahiro Yamada <yamada.masahiro@...ionext.com> wrote on Fri,  8 Feb
> 2019 17:08:50 +0900:
>
> > Currently, wait_for_completion_timeout() is always passed in the
> > hard-coded msec_to_jiffies(1000). There is no specific reason for
> > 1000 msec, but I just chose it long enough.
> >
> > With the exec_op() conversion, NAND_OP_WAITRDY_INSTR provides more
> > precise timeout value, depending on the preceding command. Let's use
> > it to bail out earlier in error case.
>
> I'm not sure using 10ms instead of 1000ms is relevant in the below
> cases, 10ms is rather short for an IRQ, if your system is under load
> you might end up with a timeout, not because the right IRQ did not
> fire, but because the handler was not executed yet (it happened to me
> in the marvell_nand.c driver recently).


Good point.
Since Linux is not RT-OS, there is no defined worst-case time
until the handler is invoked.


I will add the following to denali_wait_for_irq().

        /* Prolong the IRQ wait time in case the system is under heavy load. */
        timeout_ms += 100;





> Also, would you mind using a define instead of hardcoding '1000'?


I do not think this is worth doing.




> >
> > I am still keeping the hard-coded values for other higher level hooks
> > such as page_read, page_write, etc. We know the value of tR, tPROG, but
> > we have unknowledge about the data transfer speed of the DMA engine.
> >
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@...ionext.com>
>
>
> Thanks,
> Miquèl



-- 
Best Regards
Masahiro Yamada