From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-f68.google.com ([209.85.210.68]:35504 "EHLO mail-ot1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726644AbeLADEr (ORCPT ); Fri, 30 Nov 2018 22:04:47 -0500 Received: by mail-ot1-f68.google.com with SMTP id 81so5531596otj.2 for ; Fri, 30 Nov 2018 07:55:00 -0800 (PST) MIME-Version: 1.0 References: <154353682674.1676897.15440708268545845062.stgit@dwillia2-desk3.amr.corp.intel.com> <20181130154902.GL10377@bombadil.infradead.org> In-Reply-To: <20181130154902.GL10377@bombadil.infradead.org> From: Dan Williams Date: Fri, 30 Nov 2018 07:54:49 -0800 Message-ID: Subject: Re: [PATCH] dax: Fix Xarray conversion of dax_unlock_mapping_entry() To: Matthew Wilcox Cc: linux-nvdimm , Jan Kara , linux-fsdevel , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Fri, Nov 30, 2018 at 7:49 AM Matthew Wilcox wrote: > > On Thu, Nov 29, 2018 at 04:13:46PM -0800, Dan Williams wrote: > > Internal to dax_unlock_mapping_entry(), dax_unlock_entry() is used to > > store a replacement entry in the Xarray at the given xas-index with the > > DAX_LOCKED bit clear. When called, dax_unlock_entry() expects the unlocked > > value of the entry relative to the current Xarray state to be specified. > > > > In most contexts dax_unlock_entry() is operating in the same scope as > > the matched dax_lock_entry(). However, in the dax_unlock_mapping_entry() > > case the implementation needs to recall the original entry. In the case > > where the original entry is a 'pmd' entry it is possible that the pfn > > performed to do the lookup is misaligned to the value retrieved in the > > Xarray. > > So far, dax_unlock_mapping_entry only has the one caller. I'd rather we > returned the 'entry' to the caller, then had them pass it back to the > unlock function. That matches the flow in the rest of DAX and doesn't > pose an undue burden to the caller. > > I plan to reclaim the DAX_LOCK bit (and the DAX_EMPTY bit for that > matter), instead using a special DAX_LOCK value. DAX is almost free of > assumptions about the other bits in a locked entry, and this will remove > the assuption that there's a PMD bit in the entry. > > How does this look? > Looks good to me, although can we make that cookie an actual type? I think it's mostly ok to pass around (void *) for 'entry' inside of fs/dax.c, but once an entry leaves that file I'd like it to have an explicit type to catch people that might accidentally pass a (struct page *) to the unlock routine.