Re: [PATCH] raid5: use memalloc_noio_save()/restore in resize_chunks()

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

On 15.04.20 13:48, Michal Hocko wrote:
> On Thu 09-04-20 23:38:13, Guoqing Jiang wrote:
> [...]
>> Not know memalloc_noio_{save,restore} well, but I guess it is better
>> to use them to mark a small scope, just my two cents.
> This would go against the intentio of the api. It is really meant to
> define reclaim recursion problematic scope.

Well, in current proposal, the scope is just when 
scribble_allo/kvmalloc_array is called.

memalloc_noio_save
scribble_allo/kvmalloc_array
memalloc_noio_restore

With the new proposal, the marked scope would be bigger than current one 
since there
are lots of places call mddev_suspend/resume.

mddev_suspend
memalloc_noio_save
...
memalloc_noio_restore
mddev_resume

IMHO, if the current proposal works then what is the advantage to 
increase the scope.
If all the callers of mddev_suspend/resume could suffer from the 
deadlock issue due to
recursing fs io, then it is definitely need to use the new proposal.

> If there is a clear entry point where any further allocation recursing to FS/IO could deadlock
> then it should be used at that level.

Agree.

At the end of mddev_suspend, I guess there is no FS/IO could happen to 
the array,
because mddev->suspended++ and quiesce(mddev, 1) are called previously in
mddev_suspend. And it makes me curious to go through to the call chain:

layout_store / chunk_size_store / update_raid_disks / update_array_info 
/ md_check_recovery
         => pers->check_reshape => raid{5,6}_check_reshape => check_reshape
         => resize_chunks => scribble_alloc(..., GFP_NOIO)

Perhaps I missed something, but seems all the 5 original callers are not 
called between
mddev_suspend and mddev_resume, if so, then with the new proposal, 
scribble_alloc()
could not be protected by the memalloc_noio_{save,restore}.

> This might be a lock which is
> taken from the reclaim or like this case a device is suspended and no IO
> is processed so anything that would wait for an IO or rely on IO making
> progress in the reclaim path would deadlock.

Thanks for teaching.

> Please have a look at Documentation/core-api/gfp_mask-from-fs-io.rst
> and let me know is something could be made more clear or explicit.
> I am more than happy to improve the documentation.

Thanks again for your lighting, will read it.

Regards,
Guoqing