| Message ID | f748aef6-9def-bea1-de7f-5ff88d7b3285@virtuozzo.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: use in_task() in __gfp_pfmemalloc_flags() | expand |
On Mon, 9 Aug 2021 11:23:29 +0300 Vasily Averin <vvs@virtuozzo.com> wrote: > obsoleted in_interrupt() include task context with disabled BH, > it's better to use in_task() instead. Are these just blind conversions, or have you verified in each case that it is correct to newly take these code paths inside local_bh_disable()? If "yes" then please provide the reasoning in each changelog?
On 8/9/21 9:24 PM, Andrew Morton wrote: > On Mon, 9 Aug 2021 11:23:29 +0300 Vasily Averin <vvs@virtuozzo.com> wrote: > >> obsoleted in_interrupt() include task context with disabled BH, >> it's better to use in_task() instead. > > Are these just blind conversions, or have you verified in each case > that it is correct to newly take these code paths inside > local_bh_disable()? > > If "yes" then please provide the reasoning in each changelog? I cannot say that it is blind conversion. In all fixed patches in_interrupt() is used to identify task context and access current. it is incorrect because in_interrupt() include task context with disabled BH Recently I hit some bug and spend a lot of time for its investigation. Right now I investigate some other issue in neighborhood, noticed sadly familiar pattern and decided fix them too. Then noticed few more similar places. In fact I would like to replace all such places in mm in one patch. Do you want to consider each of these cases separately? In this particular case in_interrupt() is used to prevent current access. I think it is safe to look at current->flags and call oom_reserves_allowed() for tasks with disabled BH and I believe this should provide more correct result. Historically this check was added to handle interrupt context and said nothing special about task context with disabled BH. Could you please clarify wich kind of arguments should I provide? Thank you, Vasily Averin
diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 856b175..4291639 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -4733,7 +4733,7 @@ static inline int __gfp_pfmemalloc_flags(gfp_t gfp_mask) return ALLOC_NO_WATERMARKS; if (in_serving_softirq() && (current->flags & PF_MEMALLOC)) return ALLOC_NO_WATERMARKS; - if (!in_interrupt()) { + if (in_task()) { if (current->flags & PF_MEMALLOC) return ALLOC_NO_WATERMARKS; else if (oom_reserves_allowed(current))
obsoleted in_interrupt() include task context with disabled BH, it's better to use in_task() instead. Signed-off-by: Vasily Averin <vvs@virtuozzo.com> --- mm/page_alloc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)