Linux-Fsdevel Archive on lore.kernel.org
 help / color / Atom feed
From: Dan Williams <dan.j.williams@intel.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: linux-nvdimm <linux-nvdimm@lists.01.org>, Jan Kara <jack@suse.cz>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] dax: Fix Xarray conversion of dax_unlock_mapping_entry()
Date: Fri, 30 Nov 2018 07:54:49 -0800
Message-ID: <CAPcyv4iE375mP7eJP4nb76shWNaATy6FcZYi800wZ3iAJE8G8g@mail.gmail.com> (raw)
In-Reply-To: <20181130154902.GL10377@bombadil.infradead.org>

On Fri, Nov 30, 2018 at 7:49 AM Matthew Wilcox <willy@infradead.org> 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.

  reply index

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-30  0:13 Dan Williams
2018-11-30 15:49 ` Matthew Wilcox
2018-11-30 15:54   ` Dan Williams [this message]
2018-11-30 16:24     ` Matthew Wilcox
2018-11-30 16:33       ` Dan Williams
2018-11-30 17:01         ` Dan Williams
2018-11-30 19:50           ` Matthew Wilcox
2018-11-30 20:05             ` Dan Williams
2018-12-04  3:33               ` Dan Williams
2018-12-05  1:34                 ` Matthew Wilcox
2018-12-05  6:11                   ` Dan Williams
2018-12-05  9:22                     ` Jan Kara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAPcyv4iE375mP7eJP4nb76shWNaATy6FcZYi800wZ3iAJE8G8g@mail.gmail.com \
    --to=dan.j.williams@intel.com \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Fsdevel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-fsdevel/0 linux-fsdevel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-fsdevel linux-fsdevel/ https://lore.kernel.org/linux-fsdevel \
		linux-fsdevel@vger.kernel.org
	public-inbox-index linux-fsdevel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-fsdevel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git