On Fri, Oct 22, 2021 at 01:37:28AM +0000, Jane Chu wrote:

On 10/21/2021 4:31 AM, Christoph Hellwig wrote:

quoted

Looking over the series I have serious doubts that overloading the
slow path clear poison operation over the fast path read/write
path is such a great idea.

Why would data recovery after a media error ever be considered a
fast/hot path?  A normal read/write to a fsdax file would not pass the
flag, which skips the poison checking with whatever MCE consequences
that has, right?

pwritev2(..., RWF_RECOVER_DATA) should be infrequent enough that
carefully stepping around dax_direct_access only has to be faster than
restoring from backup, I hope?

Understood, sounds like a concern on principle. But it seems to me
that the task of recovery overlaps with the normal write operation
on the write part. Without overloading some write operation for
'recovery', I guess we'll need to come up with a new userland
command coupled with a new dax API ->clear_poison and propagate the
new API support to each dm targets that support dax which, again,
is an idea that sounds too bulky if I recall Dan's earlier rejection
correctly.

It is in my plan though, to provide pwritev2(2) and preadv2(2) man pages
with description about the RWF_RECOVERY_DATA flag and specifically not
recommending the flag for normal read/write operation - due to potential
performance impact from searching badblocks in the range.

Yes, this will help much. :)

That said, the badblock searching is turned on only if the pmem device
contains badblocks(i.e. bb->count > 0), otherwise, the performance
impact is next to none. And once the badblock search starts,
it is a binary search over user provided range. The unwanted impact
happens to be the case when the pmem device contains badblocks
that do not fall in the user specified range, and in that case, the
search would end in O(1).

(I wonder about improving badblocks to be less sector-oriented and not
have that weird 16-records-max limit, but that can be a later
optimization.)

--D

Thanks!
-jane