From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760296AbcLURvd (ORCPT ); Wed, 21 Dec 2016 12:51:33 -0500 Received: from mga03.intel.com ([134.134.136.65]:52094 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759465AbcLURvb (ORCPT ); Wed, 21 Dec 2016 12:51:31 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,384,1477983600"; d="scan'208";a="21290633" Date: Wed, 21 Dec 2016 10:51:18 -0700 From: Ross Zwisler To: Jan Kara Cc: Dan Williams , akpm@linux-foundation.org, Matthew Wilcox , Dave Chinner , linux-kernel@vger.kernel.org, Dave Hansen , Alexander Viro , linux-fsdevel@vger.kernel.org, Ross Zwisler , Christoph Hellwig Subject: Re: [PATCH] dax: kill uml support Message-ID: <20161221175118.GA4716@linux.intel.com> Mail-Followup-To: Ross Zwisler , Jan Kara , Dan Williams , akpm@linux-foundation.org, Matthew Wilcox , Dave Chinner , linux-kernel@vger.kernel.org, Dave Hansen , Alexander Viro , linux-fsdevel@vger.kernel.org, Christoph Hellwig References: <148228426023.2477.2662330241414304847.stgit@dwillia2-desk3.amr.corp.intel.com> <20161221085347.GB4756@quack2.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161221085347.GB4756@quack2.suse.cz> User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Dec 21, 2016 at 09:53:47AM +0100, Jan Kara wrote: > On Tue 20-12-16 17:37:40, Dan Williams wrote: > > The lack of common transparent-huge-page helpers for UML is becoming > > increasingly painful for fs/dax.c now that it is growing more pmd > > functionality. Add UML to the list of unsupported architectures, and > > clean up no-longer-necessary ifdef as a result. > ... > > diff --git a/fs/dax.c b/fs/dax.c > > index ddcddfeaa03b..86df835783ea 100644 > > --- a/fs/dax.c > > +++ b/fs/dax.c > > @@ -710,8 +710,7 @@ static void dax_mapping_entry_mkclean(struct address_space *mapping, > > if (follow_pte_pmd(vma->vm_mm, address, &ptep, &pmdp, &ptl)) > > continue; > > > > - if (pmdp) { > > -#ifdef CONFIG_FS_DAX_PMD > > + if (pmdp && IS_ENABLED(CONFIG_FS_DAX_PMD)) { > > pmd_t pmd; > > So I was under the impression that pmdp can never be != NULL when > CONFIG_FS_DAX_PMD is disabled. Otherwise Ross' patch would leave ptl locked > in that case... Did I miss something or we can just remove IS_ENABLED() > check? We need the IS_ENABLED() check to prevent a different build error. The #ifdef was there to prevent compile time errors where pmd_pfn(), pmd_write(), etc were all undefined symbols because they aren't defined in the arch/um headers. For x86_64 we can get linker errors if CONFIG_TRANSPARENT_HUGEPAGE isn't set: fs/built-in.o: In function `dax_writeback_mapping_range.part.27': dax.c:(.text+0x4ce28): undefined reference to `pmdp_huge_clear_flush' In this config we do have a prototype for pmdp_huge_clear_flush() in include/asm-generic/pgtable.h so it doesn't show up as an undefined symbol, but the implementation in mm/pgtable-generic.c is in a #ifdef CONFIG_TRANSPARENT_HUGEPAGE block. The IS_ENABLED() lets the compiler optimize out the code that calls pmdp_huge_clear_flush() so it doesn't try and resolve that symbol at link time. (If any of this sounds incorrect or I'm using the wrong terminology for anything, please correct me!) You're right that 'pmdp' should always be NULL if CONFIG_FS_DAX_PMD isn't defined.