On 10/29/2021 4:46 AM, Pavel Begunkov wrote: > On 10/28/21 23:59, Dave Chinner wrote: > [...] >>>> Well, my point is doing recovery from bit errors is by definition not >>>> the fast path. Which is why I'd rather keep it away from the pmem >>>> read/write fast path, which also happens to be the (much more >>>> important) >>>> non-pmem read/write path. >>> >>> The trouble is, we really /do/ want to be able to (re)write the failed >>> area, and we probably want to try to read whatever we can. Those are >>> reads and writes, not {pre,f}allocation activities. This is where Dave >>> and I arrived at a month ago. >>> >>> Unless you'd be ok with a second IO path for recovery where we're >>> allowed to be slow? That would probably have the same user interface >>> flag, just a different path into the pmem driver. >> >> I just don't see how 4 single line branches to propage RWF_RECOVERY >> down to the hardware is in any way an imposition on the fast path. >> It's no different for passing RWF_HIPRI down to the hardware *in the >> fast path* so that the IO runs the hardware in polling mode because >> it's faster for some hardware. > > Not particularly about this flag, but it is expensive. Surely looks > cheap when it's just one feature, but there are dozens of them with > limited applicability, default config kernels are already sluggish > when it comes to really fast devices and it's not getting better. > Also, pretty often every of them will add a bunch of extra checks > to fix something of whatever it would be. > > So let's add a bit of pragmatism to the picture, if there is just one > user of a feature but it adds overhead for millions of machines that > won't ever use it, it's expensive. > > This one doesn't spill yet into paths I care about, but in general > it'd be great if we start thinking more about such stuff instead of > throwing yet another if into the path, e.g. by shifting the overhead > from linear to a constant for cases that don't use it, for instance > with callbacks or bit masks. May I ask what solution would you propose for pmem recovery that satisfy the requirement of binding poison-clearing and write in one operation? thanks! -jane > >> IOWs, saying that we shouldn't implement RWF_RECOVERY because it >> adds a handful of branches the fast path is like saying that we >> shouldn't implement RWF_HIPRI because it slows down the fast path >> for non-polled IO.... >> >> Just factor the actual recovery operations out into a separate >> function like: >