linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Pavel Tatashin <pasha.tatashin@oracle.com>
To: Michal Hocko <mhocko@kernel.org>
Cc: Steve Sistare <steven.sistare@oracle.com>,
	Daniel Jordan <daniel.m.jordan@oracle.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Mel Gorman <mgorman@techsingularity.net>,
	Linux Memory Management List <linux-mm@kvack.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Vlastimil Babka <vbabka@suse.cz>,
	Bharata B Rao <bharata@linux.vnet.ibm.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	mingo@redhat.com, hpa@zytor.com, x86@kernel.org,
	dan.j.williams@intel.com, kirill.shutemov@linux.intel.com,
	bhe@redhat.com
Subject: Re: [PATCH v3 1/4] mm/memory_hotplug: enforce block size aligned range check
Date: Thu, 15 Feb 2018 08:36:00 -0500	[thread overview]
Message-ID: <CAOAebxvF6mxDb4Ub02F0B9TEMRJUG0UGrKJ6ypaMGcje80cy6w@mail.gmail.com> (raw)
In-Reply-To: <20180215113407.GB7275@dhcp22.suse.cz>

Hi Michal,

Thank you very much for your reviews and for Acking this patch.

>
> The whole memblock != section_size sucks! It leads to corner cases like
> you see. There is no real reason why we shouldn't be able to to online
> 2G unaligned memory range. Failing for that purpose is just wrong. The
> whole thing is just not well thought through and works only for well
> configured cases.

Hotplug operates over memory blocks, and it seems that conceptually
memory blocks are OK: their sizes are defined by arch, and may
represent a pluggable dimm (on virtual machines it is a different
story though). If we forced memory blocks to be equal to section size,
that would force us to handle millions of memory devices in sysfs,
which would not scale well.

>
> Your patch doesn't address the underlying problem.

What is the underlying problem? The hotplug operation was allowed, but
we ended up with half populated memory block, which is broken. The
patch solves this problem by making sure that this is never the case
for any arch, no matter what block size is defined as unit of
hotplugging.

> On the other hand, it
> is incorrect to check memory section here conceptually because this is
> not a hotplug unit as you say so I am OK with the patch regardless. It
> deserves a big fat TODO to fix this properly at least. I am not sure why
> we insist on the alignment in the first place. All we should care about
> is the proper memory section based range. The code is crap and it
> assumes pageblock start aligned at some places but there shouldn't be
> anything fundamental to change that.

So, if I understand correctly, ideally you would like to redefine unit
of memory hotplug to be equal to section size?

>
>> Signed-off-by: Pavel Tatashin <pasha.tatashin@oracle.com>
>
> Acked-by: Michal Hocko <mhocko@suse.com>

Thank you,
Pavel

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2018-02-15 13:36 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-13 19:31 [PATCH v3 0/4] optimize memory hotplug Pavel Tatashin
2018-02-13 19:31 ` [PATCH v3 1/4] mm/memory_hotplug: enforce block size aligned range check Pavel Tatashin
2018-02-15 11:34   ` Michal Hocko
2018-02-15 13:36     ` Pavel Tatashin [this message]
2018-02-15 14:40       ` Michal Hocko
2018-02-15 15:05         ` Pavel Tatashin
2018-02-13 19:31 ` [PATCH v3 2/4] x86/mm/memory_hotplug: determine block size based on the end of boot memory Pavel Tatashin
2018-02-15 11:37   ` Michal Hocko
2018-02-15 13:39     ` Pavel Tatashin
2018-02-15 19:00       ` Ingo Molnar
2018-02-13 19:31 ` [PATCH v3 3/4] mm: uninitialized struct page poisoning sanity checking Pavel Tatashin
2018-02-15 11:53   ` Michal Hocko
2018-02-15 13:41     ` Pavel Tatashin
2018-02-13 19:31 ` [PATCH v3 4/4] mm/memory_hotplug: optimize memory hotplug Pavel Tatashin
2018-02-15 12:43   ` Michal Hocko
2018-02-15 13:46     ` Pavel Tatashin
2018-02-13 21:53 ` [PATCH v3 0/4] " Andrew Morton
2018-02-14  8:09   ` Ingo Molnar
2018-02-14 14:14     ` Pavel Tatashin

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=CAOAebxvF6mxDb4Ub02F0B9TEMRJUG0UGrKJ6ypaMGcje80cy6w@mail.gmail.com \
    --to=pasha.tatashin@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=bharata@linux.vnet.ibm.com \
    --cc=bhe@redhat.com \
    --cc=dan.j.williams@intel.com \
    --cc=daniel.m.jordan@oracle.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hpa@zytor.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@techsingularity.net \
    --cc=mhocko@kernel.org \
    --cc=mingo@redhat.com \
    --cc=steven.sistare@oracle.com \
    --cc=tglx@linutronix.de \
    --cc=vbabka@suse.cz \
    --cc=x86@kernel.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