Re: [PATCH RFC] mtd: spinand: winbond: enable continuous read for W25N04LW

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Dominique,

>> > Fun fact:
>> > nanddump -C is about 9% faster than nanddump on large data (tried 10MB)
>> > even if continuous read is not supported.
>> 
>> That is strange. Is this really reproducible? Can you disable CPU PM and
>> try again? There should be no impact if continuous read is disabled.
>
> This system has no knob that I'm aware of (no cpufreq in
> /sys/devices/system/cpu at least), but I've shoved the IRQs aside except
> for the spi ones and it appears pretty stable (mtd1 is a 8MB partition)
>
> ------
> localhost:/mnt/mtd-utils# taskset -c 1 hyperfine --warmup 3 -r 20  './nanddump -f /dev/null /dev/mtd1' './nanddump -C -
> f /dev/null /dev/mtd1'
> Benchmark 1: ./nanddump -f /dev/null /dev/mtd1
>   Time (mean ± σ):     511.1 ms ±   0.5 ms    [User: 7.8 ms, System: 419.8 ms]
>   Range (min … max):   510.4 ms … 512.0 ms    20 runs
>  
> Benchmark 2: ./nanddump -C -f /dev/null /dev/mtd1
>   Time (mean ± σ):     480.9 ms ±   0.9 ms    [User: 0.5 ms, System: 399.6 ms]
>   Range (min … max):   480.1 ms … 483.3 ms    20 runs
>  
>   Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.
>  
> Summary
>   ./nanddump -C -f /dev/null /dev/mtd1 ran
>     1.06 ± 0.00 times faster than ./nanddump -f /dev/null /dev/mtd1
> ------
> (this is on our 6.12 tree and not on the 7.2-rc I was on earlier, so
> ymmv, but iirc the 9% figure was taken on 7.2 with hyperfine so this
> should be reproducible with the latest and greatest)

No big change is expected between those versions, except possible
(important) fixes. Make sure you have them.

> I have much more important things to do so I obviously had to look
> further into this mystery :-), but I didn't see anything obvious...

:-)

> For some reason the contiguous read variant seems to spend less time in
> spinand_wait() between the load page and read from cache op (looking at
> a flamegraph), but dumping the actual ops used I see very similar
> sequences of load page (0x13), poll status (0xf), dirmap_read (0xeb),
> so that doesn't explain the difference...

I do not understand why. Did you compare the output? Do you get the same
correct data in both cases?

Maybe there is some kind of runtime PM getting in the way?

> The main difference I see is that nanddump -C passes a much larger
> buffer to read (observable with strace -c), so loops for reading inside
> the kernel, and without -C loops in userspace: I do not see the syscall
> overhead in my perf record flamegraph but perhaps that changes the
> timing just enough for the nand to behave differently or something like
> that?

While nanddump -C is nice, as you said it behaves differently. I would
instead suggest to run flash_speed which has a continuous read
option. You should observe no difference between the different runs.

Then you can hack into the core, enable continuous reads despite the use
of dirmaps (as a hack) and then observe a difference. You may or may not
get correct data depending on whether the controller may deassert the
CS, but we have no control over this, but that doesn't matter much for
the current investigation.

Thanks,
Miquèl