On Wed 15-04-20 16:10:08, Guoqing Jiang wrote:
> 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.
The advantage is twofold. It serves the documentation purpose because it
is clear _what_ and _why_ is the actual allocation restricted context.
In this case mddev_{suspend,resume} because XYZ and you do not have to
care about __GFP_IO for _any_ allocation inside 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.
Why some of them wouldn't? Isn't the mddev_suspend going to block any IO
on the device? The thing is that you cannot tell which devices the
allocator can issue IO for therefore GFP_NOIO is a global flag. Please
also note that the scope API is bound to a process context so it only
affects the current execution.
--
Michal Hocko
SUSE Labs
