On 12/13/21 3:43 PM, Vishal Verma wrote:
>
> On 12/12/21 10:56 PM, Song Liu wrote:
>> On Fri, Dec 10, 2021 at 10:26 AM Vishal Verma <vverma@xxxxxxxxxxxxxxxx> wrote:
>>>
>>> On 12/9/21 7:16 PM, Song Liu wrote:
>>>> On Wed, Nov 10, 2021 at 10:15 AM Vishal Verma <vverma@xxxxxxxxxxxxxxxx> wrote:
>>>>> Returns EAGAIN in case the raid456 driver would block
>>>>> waiting for situations like:
>>>>>
>>>>> - Reshape operation,
>>>>> - Discard operation.
>>>>>
>>>>> Signed-off-by: Vishal Verma <vverma@xxxxxxxxxxxxxxxx>
>>>>> ---
>>>>> drivers/md/raid5.c | 14 ++++++++++++++
>>>>> 1 file changed, 14 insertions(+)
>>>>>
>>>>> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
>>>>> index 9c1a5877cf9f..fa64ee315241 100644
>>>>> --- a/drivers/md/raid5.c
>>>>> +++ b/drivers/md/raid5.c
>>>>> @@ -5710,6 +5710,11 @@ static void make_discard_request(struct mddev *mddev, struct bio *bi)
>>>>> int d;
>>>>> again:
>>>>> sh = raid5_get_active_stripe(conf, logical_sector, 0, 0, 0);
>>>>> + /* Bail out if REQ_NOWAIT is set */
>>>>> + if (bi->bi_opf & REQ_NOWAIT) {
>>>>> + bio_wouldblock_error(bi);
>>>>> + return;
>>>>> + }
>>>> This is not right. raid5_get_active_stripe() gets refcount on the sh,
>>>> we cannot simply
>>>> return here. I think we need the logic after raid5_release_stripe()
>>>> and before schedule().
>>>>
>>>>> prepare_to_wait(&conf->wait_for_overlap, &w,
>>>>> TASK_UNINTERRUPTIBLE);
>>>>> set_bit(R5_Overlap, &sh->dev[sh->pd_idx].flags);
>>>>> @@ -5820,6 +5825,15 @@ static bool raid5_make_request(struct mddev *mddev, struct bio * bi)
>>>>> bi->bi_next = NULL;
>>>>>
>>>>> md_account_bio(mddev, &bi);
>>>>> + /* Bail out if REQ_NOWAIT is set */
>>>>> + if (bi->bi_opf & REQ_NOWAIT &&
>>>>> + conf->reshape_progress != MaxSector &&
>>>>> + mddev->reshape_backwards
>>>>> + ? logical_sector < conf->reshape_safe
>>>>> + : logical_sector >= conf->reshape_safe) {
>>>>> + bio_wouldblock_error(bi);
>>>>> + return true;
>>>>> + }
>>>> This is also problematic, and is the trigger of those error messages.
>>>> We only want to trigger -EAGAIN when logical_sector is between
>>>> reshape_progress and reshape_safe.
>>>>
>>>> Just to clarify, did you mean doing something like:
>>>> if (bi->bi_opf & REQ_NOWAIT &&
>>>> + conf->reshape_progress != MaxSector &&
>>>> + (mddev->reshape_backwards
>>>> + ? (logical_sector > conf->reshape_progress && logical_sector < conf->reshape_safe)
>>>> + : logical_sector >= conf->reshape_safe)) {
>> I think this should be
>> : (logical_sector >= conf->reshape_safe && logical_sector <
>> conf->reshape_progress)
>
>
> if (bi->bi_opf & REQ_NOWAIT &&
> conf->reshape_progress != MaxSector &&
> (mddev->reshape_backwards
> ? (logical_sector > conf->reshape_progress &&
> logical_sector <= conf->reshape_safe)
> : (logical_sector >= conf->reshape_safe &&
> logical_sector < conf->reshape_progress))) {
> bio_wouldblock_error(bi);
> return true;
> }
>
> After making this change along with other changes, I ran some tests with
> 100% reads, 70%read30%writes and 100% writes on a clean raid5 array.
>
> Unfortunately, I ran into this following task hung with 100% writes
> (with both libaio and io_uring):
>
> [21876.856692] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs"
> disables this message.
> [21876.864518] task:md5_raid5 state:D stack: 0 pid:11675
> ppid: 2 flags:0x00004000
> [21876.864522] Call Trace:
> [21876.864526] __schedule+0x2d4/0x970
> [21876.864532] ? wbt_cleanup_cb+0x20/0x20
> [21876.864535] schedule+0x4e/0xb0
> [21876.864537] io_schedule+0x3f/0x70
> [21876.864539] rq_qos_wait+0xb9/0x130
> [21876.864542] ? sysv68_partition+0x280/0x280
> [21876.864543] ? wbt_cleanup_cb+0x20/0x20
> [21876.864545] wbt_wait+0x92/0xc0
> [21876.864546] __rq_qos_throttle+0x25/0x40
> [21876.864548] blk_mq_submit_bio+0xc6/0x5d0
> [21876.864551] ? submit_bio_checks+0x39e/0x5f0
> [21876.864554] __submit_bio+0x1bc/0x1d0
> [21876.864555] ? kmem_cache_free+0x378/0x3c0
> [21876.864558] ? mempool_free_slab+0x17/0x20
> [21876.864562] submit_bio_noacct+0x256/0x2a0
> [21876.864565] 0xffffffffc01fa6d9
> [21876.864568] ? 0xffffffffc01f5d01
> [21876.864569] raid5_get_active_stripe+0x16c0/0x3e00 [raid456]
> [21876.864571] ? __wake_up_common_lock+0x8a/0xc0
> [21876.864575] raid5_get_active_stripe+0x2839/0x3e00 [raid456]
> [21876.864577] raid5_get_active_stripe+0x2d6e/0x3e00 [raid456]
> [21876.864579] md_thread+0xae/0x170
> [21876.864581] ? wait_woken+0x60/0x60
> [21876.864582] ? md_start_sync+0x60/0x60
> [21876.864584] kthread+0x127/0x150
> [21876.864586] ? set_kthread_struct+0x40/0x40
> [21876.864588] ret_from_fork+0x1f/0x30
What kernel base are you using for your patches?
--
Jens Axboe
