On Fri 23-02-18 00:56:26, Stephen Rothwell wrote: > Hi Michal, > > On Thu, 22 Feb 2018 08:11:00 +0100 Michal Hocko <mhocko@xxxxxxxxxx> wrote: > > > > This is interesting. I thought that IS_ENABLED(CONFIG_HAVE_MEMBLOCK) > > would have the same meaning as ifdef CONFIG_HAVE_MEMBLOCK so the branch > > will never be considered. If that is not the case then I would rather > > reintroduce that ifdef. We already have those in the function anyway. > > Actually, you don't need a definition of memblock_next_valid_pfn() in the > !CONFIG_HAVE_MEMBLOCK case, just a declaration, so the minimal fix is > to move the declaration out of the #ifdef CONFIG_HAVE_MEMBLOCK in the > header file. You are right. > That way if there is any use of memblock_next_valid_pfn() > introduced that is no guarded by IS_ENABLED(CONFIG_HAVE_MEMBLOCK) the > build will fail to link. I like IS_ENABLED() being used wherever > possible because it allows us better compiler coverage (in the face of > CONFIG options) even if the compiler then elides the actual code. It > also breaks the code up less than #ifdef's. > > Your choice, of course. The function already has those ugly ifdefs so I would keep it consistent. Deuglyfying it would need a bigger stick. -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-next" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
