linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Vasily Averin <vvs@virtuozzo.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel@openvz.org
Subject: Re: [PATCH] mm: use in_task() in __gfp_pfmemalloc_flags()
Date: Tue, 10 Aug 2021 14:04:07 +0300	[thread overview]
Message-ID: <8bbc1658-03b2-f64e-d11a-153da3eb723a@virtuozzo.com> (raw)
In-Reply-To: <20210809112448.3d7d8f2522e18e75ba6e31c0@linux-foundation.org>

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


      reply	other threads:[~2021-08-10 11:04 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-09  8:23 Vasily Averin
2021-08-09 18:24 ` Andrew Morton
2021-08-10 11:04   ` Vasily Averin [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8bbc1658-03b2-f64e-d11a-153da3eb723a@virtuozzo.com \
    --to=vvs@virtuozzo.com \
    --cc=akpm@linux-foundation.org \
    --cc=kernel@openvz.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox