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