Re: your mail

From	Michal Hocko <mhocko@kernel.org>
Newsgroups	linux.kernel
Subject	Re: your mail
Date	2017-04-27 17:20 +0200
Message-ID	<tATn5-2pt-33@gated-at.bofh.it> (permalink)
References	(5 earlier) <tzBiy-8qY-3@gated-at.bofh.it> <tzH4B-4c8-1@gated-at.bofh.it> <tzYRP-7bL-3@gated-at.bofh.it> <tArKb-tv-61@gated-at.bofh.it> <tAH2x-2nz-5@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Thu 27-04-17 11:08:38, Joonsoo Kim wrote:
> On Wed, Apr 26, 2017 at 11:19:06AM +0200, Michal Hocko wrote:
> > > > [...]
> > > > 
> > > > > > You are trying to change a semantic of something that has a well defined
> > > > > > meaning. I disagree that we should change it. It might sound like a
> > > > > > simpler thing to do because pfn walkers will have to be checked but what
> > > > > > you are proposing is conflating two different things together.
> > > > > 
> > > > > I don't think that *I* try to change the semantic of pfn_valid().
> > > > > It would be original semantic of pfn_valid().
> > > > > 
> > > > > "If pfn_valid() returns true, we can get proper struct page and the
> > > > > zone information,"
> > > > 
> > > > I do not see any guarantee about the zone information anywhere. In fact
> > > > this is not true with the original implementation as I've tried to
> > > > explain already. We do have new pages associated with a zone but that
> > > > association might change during the online phase. So you cannot really
> > > > rely on that information until the page is online. There is no real
> > > > change in that regards after my rework.
> > > 
> > > I know that what you did doesn't change thing much. What I try to say
> > > is that previous implementation related to pfn_valid() in hotplug is
> > > wrong. Please do not assume that hotplug implementation is correct and
> > > other pfn_valid() users are incorrect. There is no design document so
> > > I'm not sure which one is correct but assumption that pfn_valid() user
> > > can access whole the struct page information makes much sense to me.
> > 
> > Not really. E.g. ZONE_DEVICE pages are never online AFAIK. I believe we
> > still need pfn_valid to work for those pfns. Really, pfn_valid has a
> 
> It's really contrary example to your insist. They requires not only
> struct page but also other information, especially, the zone index.
> They checks zone idx to know whether this page is for ZONE_DEVICE or not.

Yes and they guarantee this association is true. Without memory onlining
though. This memory is never online for anybody who is asking.

[...]

> I think that I did my best to explain my reasoning. It seems that we
> cannot agree with each other so it's better for some others to express
> their opinion to this problem. I will stop this discussion from now
> on.

I _do_ appreciate your feedback and if the general consensus is to
modify pfn_valid I can go that direction but my gut feeling tells me
that conflating "existing struct page" test and "fully online and
initialized" one is a wrong thing to do.
-- 
Michal Hocko
SUSE Labs

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

Re: your mail Joonsoo Kim <js1304@gmail.com> - 2017-04-25 05:00 +0200
  Re: your mail Michal Hocko <mhocko@kernel.org> - 2017-04-26 11:50 +0200
    Re: your mail Joonsoo Kim <js1304@gmail.com> - 2017-04-27 04:10 +0200
      Re: your mail Michal Hocko <mhocko@kernel.org> - 2017-04-27 17:20 +0200

csiph-web