On Aug 1, 2019, at 1:28 PM, Guilherme G. Piccoli [off-list ref] wrote:



On 31/07/2019 16:56, Song Liu wrote:

quoted

On Wed, Jul 31, 2019 at 12:54 PM Song Liu [off-list ref] wrote:

quoted

On Tue, Jul 30, 2019 at 5:31 AM Guilherme G. Piccoli
[off-list ref] wrote:

quoted

On 29/07/2019 21:08, NeilBrown wrote:

quoted

[...]

quoted

+    if (unlikely(test_bit(MD_BROKEN, &mddev->flags))) {
+            bio_io_error(bio);
+            return BLK_QC_T_NONE;
+    }

I think this should only fail WRITE requests, not READ requests.

Otherwise the patch is probably reasonable.

NeilBrown

Thanks for the feedback Neil! I thought about it; it seemed to me better
to deny/fail the reads instead of returning "wrong" reads, since a file
read in a raid0 will be incomplete if one member is missing.
But it's fine for me to change that in the next iteration of this patch.

For reads at block/page level, we will either get EIO or valid data, right?

If that's not the case, we should fail all writes.

Oops, I meant all _reads_.

Hi Song, thanks for the feedback! After changing the patch and testing a
bit, it behaves exactly as you said, we got either valid data read from
the healthy devices or -EIO for the data tentatively read from the
failed/missing array members.

Thanks for testing this out. 

So, I'll resubmit with that change. Also, I've noticed clearing the
BROKEN flag seem unnecessary, if user stops the array in order to fix
the missing member, it'll require a re-assembly and the array is gonna
work again.

Do you / Neil considers this fix relevant to md/linear too? If so, I can
also include that in the V2.

Yes, please also include fix for md/linear. 

Song