All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oscar Salvador <osalvador@suse.de>
To: David Hildenbrand <david@redhat.com>
Cc: linux-mm@kvack.org, Qian Cai <cai@lca.pw>,
	Michal Hocko <mhocko@suse.com>,
	linux-nvdimm@lists.01.org, linux-kernel@vger.kernel.org,
	Anshuman Khandual <anshuman.khandual@arm.com>
Subject: Re: [PATCH v2 2/5] mm: Teach pfn_to_online_page() to consider subsection validity
Date: Tue, 12 Jan 2021 11:48:23 +0100	[thread overview]
Message-ID: <20210112104817.GA12956@linux> (raw)
In-Reply-To: <0586c562-787c-4872-4132-18a49c3ffc8e@redhat.com>

On Tue, Jan 12, 2021 at 10:53:17AM +0100, David Hildenbrand wrote:
> That's not sufficient for alternative implementations of pfn_valid().
> 
> You still need some kind of pfn_valid(pfn) for alternative versions of
> pfn_valid(). Consider arm64 memory holes in the memmap. See their
> current (yet to be fixed/reworked) pfn_valid() implementation.
> (pfn_valid_within() is implicitly active on arm64)
> 
> Actually, I think we should add something like the following, to make
> this clearer (pfn_valid_within() is confusing)
> 
> #ifdef CONFIG_HAVE_ARCH_PFN_VALID
> 	/* We might have to check for holes inside the memmap. */
> 	if (!pfn_valid())
> 		return NULL;
> #endif

I have to confess that I was a bit confused by pfn_valid_within + HOLES_IN_ZONES
+ HAVE_ARCH_PFN_VALID.

At first I thought that we should stick with pfn_valid_within, as we also
depend on HOLES_IN_ZONES, so it could be that

 if (IS_ENABLED(CONFIG_HAVE_ARCH_PFN_VALID))
  ...

would to too much work, as if CONFIG_HOLES_IN_ZONES was not set but an arch
pfn_valid was provided, we would perform unedeed checks.
But on a closer look, CONFIG_HOLES_IN_ZONES is set by default on arm64, and
on ia64 when SPARSEMEM is set, so looks fine.


-- 
Oscar Salvador
SUSE L3
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

WARNING: multiple messages have this Message-ID (diff)
From: Oscar Salvador <osalvador@suse.de>
To: David Hildenbrand <david@redhat.com>
Cc: Dan Williams <dan.j.williams@intel.com>,
	linux-mm@kvack.org, Qian Cai <cai@lca.pw>,
	Michal Hocko <mhocko@suse.com>,
	vishal.l.verma@intel.com, linux-nvdimm@lists.01.org,
	linux-kernel@vger.kernel.org,
	Anshuman Khandual <anshuman.khandual@arm.com>
Subject: Re: [PATCH v2 2/5] mm: Teach pfn_to_online_page() to consider subsection validity
Date: Tue, 12 Jan 2021 11:48:23 +0100	[thread overview]
Message-ID: <20210112104817.GA12956@linux> (raw)
In-Reply-To: <0586c562-787c-4872-4132-18a49c3ffc8e@redhat.com>

On Tue, Jan 12, 2021 at 10:53:17AM +0100, David Hildenbrand wrote:
> That's not sufficient for alternative implementations of pfn_valid().
> 
> You still need some kind of pfn_valid(pfn) for alternative versions of
> pfn_valid(). Consider arm64 memory holes in the memmap. See their
> current (yet to be fixed/reworked) pfn_valid() implementation.
> (pfn_valid_within() is implicitly active on arm64)
> 
> Actually, I think we should add something like the following, to make
> this clearer (pfn_valid_within() is confusing)
> 
> #ifdef CONFIG_HAVE_ARCH_PFN_VALID
> 	/* We might have to check for holes inside the memmap. */
> 	if (!pfn_valid())
> 		return NULL;
> #endif

I have to confess that I was a bit confused by pfn_valid_within + HOLES_IN_ZONES
+ HAVE_ARCH_PFN_VALID.

At first I thought that we should stick with pfn_valid_within, as we also
depend on HOLES_IN_ZONES, so it could be that

 if (IS_ENABLED(CONFIG_HAVE_ARCH_PFN_VALID))
  ...

would to too much work, as if CONFIG_HOLES_IN_ZONES was not set but an arch
pfn_valid was provided, we would perform unedeed checks.
But on a closer look, CONFIG_HOLES_IN_ZONES is set by default on arm64, and
on ia64 when SPARSEMEM is set, so looks fine.


-- 
Oscar Salvador
SUSE L3


  reply	other threads:[~2021-01-12 10:48 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-12  9:34 [PATCH v2 0/5] mm: Fix pfn_to_online_page() with respect to ZONE_DEVICE Dan Williams
2021-01-12  9:34 ` Dan Williams
2021-01-12  9:34 ` [PATCH v2 1/5] mm: Move pfn_to_online_page() out of line Dan Williams
2021-01-12  9:34   ` Dan Williams
2021-01-12  9:46   ` David Hildenbrand
2021-01-12  9:46     ` David Hildenbrand
2021-01-12 10:19   ` Oscar Salvador
2021-01-12 10:19     ` Oscar Salvador
2021-01-12  9:34 ` [PATCH v2 2/5] mm: Teach pfn_to_online_page() to consider subsection validity Dan Williams
2021-01-12  9:34   ` Dan Williams
2021-01-12  9:53   ` David Hildenbrand
2021-01-12  9:53     ` David Hildenbrand
2021-01-12 10:48     ` Oscar Salvador [this message]
2021-01-12 10:48       ` Oscar Salvador
2021-01-12 22:20     ` Dan Williams
2021-01-12 22:20       ` Dan Williams
2021-01-12  9:34 ` [PATCH v2 3/5] mm: Teach pfn_to_online_page() about ZONE_DEVICE section collisions Dan Williams
2021-01-12  9:34   ` Dan Williams
2021-01-12 10:01   ` David Hildenbrand
2021-01-12 10:01     ` David Hildenbrand
2021-01-12 11:00   ` Oscar Salvador
2021-01-12 11:00     ` Oscar Salvador
2021-01-12  9:34 ` [PATCH v2 4/5] mm: Fix page reference leak in soft_offline_page() Dan Williams
2021-01-12  9:34   ` Dan Williams
2021-01-12  9:53   ` Oscar Salvador
2021-01-12  9:53     ` Oscar Salvador
2021-01-12 20:03     ` Dan Williams
2021-01-12 20:03       ` Dan Williams
2021-01-12 10:16   ` David Hildenbrand
2021-01-12 10:16     ` David Hildenbrand
2021-01-12  9:35 ` [PATCH v2 5/5] libnvdimm/namespace: Fix visibility of namespace resource attribute Dan Williams
2021-01-12  9:35   ` Dan Williams

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=20210112104817.GA12956@linux \
    --to=osalvador@suse.de \
    --cc=anshuman.khandual@arm.com \
    --cc=cai@lca.pw \
    --cc=david@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=mhocko@suse.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.