linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Dan Williams <dan.j.williams@intel.com>
To: David Hildenbrand <david@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	stable <stable@vger.kernel.org>,
	 Vishal Verma <vishal.l.verma@intel.com>,
	Pavel Tatashin <pasha.tatashin@soleen.com>,
	 Michal Hocko <mhocko@suse.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	 Linux MM <linux-mm@kvack.org>,
	 Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] mm/memory_hotplug: Fix remove_memory() lockdep splat
Date: Fri, 10 Jan 2020 09:33:35 -0800	[thread overview]
Message-ID: <CAPcyv4jVpN26RGQLRn4BewYtzHDoQfvh37DEdEBq1dd4-BP0kw@mail.gmail.com> (raw)
In-Reply-To: <fc0cfb97-5a60-7e73-4f85-d8e6947c5e28@redhat.com>

On Fri, Jan 10, 2020 at 9:29 AM David Hildenbrand <david@redhat.com> wrote:
[..]
> > So then the comment is actively misleading for that case. I would
> > expect an explicit _unlocked path for that case with a comment about
> > why it's special. Is there already a comment to that effect somewhere?
> >
>
> __add_memory() - the locked variant - is called from the same ACPI location
> either locked or unlocked. I added a comment back then after a longe
> discussion with Michal:
>
> drivers/acpi/scan.c:
>         /*
>          * Although we call __add_memory() that is documented to require the
>          * device_hotplug_lock, it is not necessary here because this is an
>          * early code when userspace or any other code path cannot trigger
>          * hotplug/hotunplug operations.
>          */
>
>
> It really is a special case, though.

That's a large comment block when we could have just taken the lock.
There's probably many other code paths in the kernel where some locks
are not necessary before userspace is up, but the code takes the lock
anyway to minimize the code maintenance burden. Is there really a
compelling reason to be clever here?


  reply	other threads:[~2020-01-10 17:33 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-10  4:30 Dan Williams
2020-01-10  9:10 ` David Hildenbrand
2020-01-10 16:42   ` Dan Williams
2020-01-10 16:54     ` David Hildenbrand
2020-01-10 16:57       ` David Hildenbrand
2020-01-10 17:24       ` Dan Williams
2020-01-10 17:29         ` David Hildenbrand
2020-01-10 17:33           ` Dan Williams [this message]
2020-01-10 17:36             ` David Hildenbrand
2020-01-10 17:39               ` Dan Williams
2020-01-10 17:42                 ` David Hildenbrand
2020-01-10 21:27                   ` Dan Williams
2020-01-24 12:45                     ` Michal Hocko
2020-01-24 18:04                       ` Dan Williams
2020-01-24 18:13                         ` David Hildenbrand
2020-01-27 13:47                         ` Michal Hocko

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=CAPcyv4jVpN26RGQLRn4BewYtzHDoQfvh37DEdEBq1dd4-BP0kw@mail.gmail.com \
    --to=dan.j.williams@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=dave.hansen@linux.intel.com \
    --cc=david@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=pasha.tatashin@soleen.com \
    --cc=stable@vger.kernel.org \
    --cc=vishal.l.verma@intel.com \
    /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