Re: [dm-devel] [PATCH 0/6] dax poison recovery with RWF_RECOVERY_DATA flag

[Jane Chu <jane.chu@oracle.com>]

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:
>