Re: "md/raid10:md5: sdf: redirecting sector 2979126480 to another mirror"

[Guoqing Jiang <guoqing.jiang@cloud.ionos.com>]

Hi Andy,

On 1/12/21 03:03, Andy Smith wrote:
> Hi Guoqing,
> 
> Thanks for following up on this. I have a couple of questions.
> 
> On Tue, Jan 12, 2021 at 01:36:55AM +0100, Guoqing Jiang wrote:
>> On 1/7/21 00:27, Andy Smith wrote:
>>> err_rdev there can only be set inside the block above that starts
>>> with:
>>>
>>>      if (r10_bio->devs[slot].rdev) {
>>>          /*
>>>           * This is an error retry, but we cannot
>>>           * safely dereference the rdev in the r10_bio,
>>>           * we must use the one in conf.
>>>
>>> …but why is this an error retry? Nothing was logged so how do I find
>>> out what the error was?
>>
>> This is because handle_read_error also calls raid10_read_request, pls see
>> commit 545250f2480 ("md/raid10: simplify handle_read_error()").
> 
> So if I understand you correctly, this is a consequence of
> raid10_read_request being reworked so that it can be called by
> handle_read_error, but in my case it is not being called by
> handle_read_error but instead raid10_make_request and is incorrectly
> going down an error path and reporting a redirected read?

Yes, that is my guess too if the message appears but there is no read 
issue from component device.

> 
>  From my stack trace it seemed that it was just
> raid10.c:__make_request that was calling raid10_read_request but I
> could not see where in __make_request the r10_bio->devs[slot].rdev
> was being set, enabling the above test to succeed. All I could see
> was a memset to 0.

IIUC, the rdev is set in raid10_run instead of before dispatch IO request.

> 
> I understand that your patch makes it so this test can no longer
> succeed when being called by __make_request, meaning that aside from
> not logging about a redirected read it will also not do the
> rcu_read_lock() / rcu_dereference() / rcu_read_unlock() that's in
> that if block. Is that a significant amount of work that is being
> needlessly done right now or is it trivial?

I think check if raid10_read_request is called from read error path is 
enough.

> 
> I'm trying to understand how big of a problem this is, beyond some
> spurious logging.
> 
> Right now when it is logging about redirecting a read, does that
> mean that it isn't actually redirecting a read? That is, when it
> says:
> 
> Jan 11 17:10:40 hostname kernel: [1318773.480077] md/raid10:md3: nvme1n1p5: redirecting sector 699122984 to another mirror
> 
> in the absence of any other error logging it is in fact its first
> try at reading and it will really be using device nvme1n1p5 (rdev)
> to satisfy that?
> 
> I suppose I am also confused why this happens so rarely. I can only
> encourage it to happen a couple of times by putting the array under
> very heavy read load, and it only seems to happen with pretty high
> performing arrays (all SSD). But perhaps that is the result of the
> rate-limited logging with pr_err_ratelimited() causing me to only
> see very few of the actual events.

If the message ("redirecting sector ...") is not comes from handle read 
err path, then I suppose the message is false alarm.


Thanks,
Guoqing