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: Baoquan He <bhe@redhat.com>,
	 Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux MM <linux-mm@kvack.org>,
	 Andrew Morton <akpm@linux-foundation.org>,
	Wei Yang <richardw.yang@linux.intel.com>
Subject: Re: [PATCH 3/7] mm/sparse.c: only use subsection map in VMEMMAP case
Date: Wed, 12 Feb 2020 07:48:15 -0800	[thread overview]
Message-ID: <CAPcyv4gLL654xtuiLJUwTGAwzZkn1CVT3i_zCjK9N2X=4nsdpw@mail.gmail.com> (raw)
In-Reply-To: <B23A05D9-40E2-4862-979D-C6DA69DDDC80@redhat.com>

On Wed, Feb 12, 2020 at 1:40 AM David Hildenbrand <david@redhat.com> wrote:
>
>
>
> > Am 11.02.2020 um 21:15 schrieb Dan Williams <dan.j.williams@intel.com>:
> >
> > On Sun, Feb 9, 2020 at 2:48 AM Baoquan He <bhe@redhat.com> wrote:
> >>
> >> Currently, subsection map is used when SPARSEMEM is enabled, including
> >> VMEMMAP case and !VMEMMAP case. However, subsection hotplug is not
> >> supported at all in SPARSEMEM|!VMEMMAP case, subsection map is unnecessary
> >> and misleading. Let's adjust code to only allow subsection map being
> >> used in SPARSEMEM|VMEMMAP case.
> >>
> >> Signed-off-by: Baoquan He <bhe@redhat.com>
> >> ---
> >> include/linux/mmzone.h |   2 +
> >> mm/sparse.c            | 231 ++++++++++++++++++++++-------------------
> >> 2 files changed, 124 insertions(+), 109 deletions(-)
> >>
> >> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> >> index 462f6873905a..fc0de3a9a51e 100644
> >> --- a/include/linux/mmzone.h
> >> +++ b/include/linux/mmzone.h
> >> @@ -1185,7 +1185,9 @@ static inline unsigned long section_nr_to_pfn(unsigned long sec)
> >> #define SUBSECTION_ALIGN_DOWN(pfn) ((pfn) & PAGE_SUBSECTION_MASK)
> >>
> >> struct mem_section_usage {
> >> +#ifdef CONFIG_SPARSEMEM_VMEMMAP
> >>        DECLARE_BITMAP(subsection_map, SUBSECTIONS_PER_SECTION);
> >> +#endif
> >
> > This was done deliberately so that the SPARSEMEM_VMEMMAP=n case ran as
> > a subset of the SPARSEMEM_VMEMMAP=y case.
> >
> > The diffstat does not seem to agree that this is any clearer:
> >
> >    124 insertions(+), 109 deletions(-)
> >
>
> I don‘t see a reason to work with subsections (+store them) if subsections are not supported.
>
> I do welcome this cleanup. Diffstats don‘t tell the whole story.

I'll take a look at the final result and see if my opinion changes,
but I just wanted to clarify upfront that making sparsemem run some of
the subsection logic was deliberate.


  parent reply	other threads:[~2020-02-12 15:48 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-09 10:48 [PATCH 0/7] mm/hotplug: Only use subsection in VMEMMAP case and fix hot add/remove failure in SPARSEMEM|!VMEMMAP case Baoquan He
2020-02-09 10:48 ` [PATCH 1/7] mm/sparse.c: Introduce new function fill_subsection_map() Baoquan He
2020-02-09 23:05   ` Wei Yang
2020-02-09 23:11     ` Wei Yang
2020-02-10  3:25     ` Baoquan He
2020-02-10  9:49   ` David Hildenbrand
2020-02-11 12:46     ` Baoquan He
2020-02-11 14:44       ` David Hildenbrand
2020-02-12 11:21         ` Baoquan He
2020-02-09 10:48 ` [PATCH 2/7] mm/sparse.c: Introduce a new function clear_subsection_map() Baoquan He
2020-02-09 23:07   ` Wei Yang
2020-02-10  3:36     ` Baoquan He
2020-02-10  6:02       ` Wei Yang
2020-02-09 10:48 ` [PATCH 3/7] mm/sparse.c: only use subsection map in VMEMMAP case Baoquan He
2020-02-09 23:23   ` Wei Yang
2020-02-11 14:43   ` David Hildenbrand
2020-02-12 11:26     ` Baoquan He
2020-02-11 20:14   ` Dan Williams
2020-02-12  9:39     ` David Hildenbrand
2020-02-12 11:20       ` Baoquan He
2020-02-12 15:48       ` Dan Williams [this message]
2020-02-09 10:48 ` [PATCH 4/7] mm/sparse.c: Use __get_free_pages() instead in populate_section_memmap() Baoquan He
2020-02-09 23:39   ` Wei Yang
2020-02-09 10:48 ` [PATCH 5/7] mm/sparse.c: update code comment about section activate/deactivate Baoquan He
2020-02-09 10:48 ` [PATCH 6/7] mm/sparsemem: pfn_to_page is not valid yet on SPARSEMEM Baoquan He
2020-02-10  3:45   ` Baoquan He
2020-02-09 10:48 ` [PATCH 7/7] mm/hotplug: fix hot remove failure in SPARSEMEM|!VMEMMAP case Baoquan He
2020-02-09 23:52   ` Wei Yang
2020-02-10  3:41     ` Baoquan He
2020-02-10  6:08       ` Wei Yang
2020-02-10  7:54         ` Baoquan He
2020-02-10 23:05           ` Wei Yang

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='CAPcyv4gLL654xtuiLJUwTGAwzZkn1CVT3i_zCjK9N2X=4nsdpw@mail.gmail.com' \
    --to=dan.j.williams@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=bhe@redhat.com \
    --cc=david@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=richardw.yang@linux.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;
as well as URLs for NNTP newsgroup(s).