From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com ([134.134.136.24]:37151 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751086AbeAYQeJ (ORCPT ); Thu, 25 Jan 2018 11:34:09 -0500 Date: Thu, 25 Jan 2018 09:34:08 -0700 From: Ross Zwisler To: Andrew Morton Cc: Ross Zwisler , Jan =?iso-8859-1?Q?H=2E_Sch=F6nherr?= , Matthew Wilcox , linux-fsdevel@vger.kernel.org Subject: Re: [PATCH] dax: Release PMD lock even when there is no PMD support in DAX Message-ID: <20180125163408.GA10655@linux.intel.com> References: <20180118133839.20587-1-jschoenh@amazon.de> <20180118162013.GB21971@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20180118162013.GB21971@linux.intel.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Thu, Jan 18, 2018 at 09:20:13AM -0700, Ross Zwisler wrote: > On Thu, Jan 18, 2018 at 02:38:39PM +0100, Jan H. Sch�nherr wrote: > > The function follow_pte_pmd() can theoretically return after having > > acquired a PMD lock, even when DAX was not compiled with > > CONFIG_FS_DAX_PMD. > > > > Release the PMD lock unconditionally. > > > > Fixes: f729c8c9b24f ("dax: wrprotect pmd_t in dax_mapping_entry_mkclean") > > Signed-off-by: Jan H. Sch�nherr > > --- > > fs/dax.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/fs/dax.c b/fs/dax.c > > index 9598159..c2ebf10 100644 > > --- a/fs/dax.c > > +++ b/fs/dax.c > > @@ -636,8 +636,8 @@ static void dax_mapping_entry_mkclean(struct address_space *mapping, > > pmd = pmd_mkclean(pmd); > > set_pmd_at(vma->vm_mm, address, pmdp, pmd); > > unlock_pmd: > > - spin_unlock(ptl); > > #endif > > + spin_unlock(ptl); > > } else { > > if (pfn != pte_pfn(*ptep)) > > goto unlock_pte; > > Sure, this seems fine to me. This seems simple and correct - you're right > that we aren't taking the PTL on the PMD conditionally based on whether > CONFIG_DAX_PMD is defined, so it doesn't make sense to release it > conditionally. I think if we ever hit this lock imbalance we're totally > insane anyway, but it the fix is correct and doesn't mess with our code flow. > > You can add: > Reviewed-by: Ross Zwisler Ah, I just realized that this patch didn't CC Andrew, and he's the one that usually takes our DAX patches. Andrew, can you pick this up? Here's the fsdevel patchwork: https://patchwork.kernel.org/patch/10173255/ Thanks, - Ross