From: Dan Williams <firstname.lastname@example.org> To: Matthew Wilcox <email@example.com> Cc: linux-nvdimm <firstname.lastname@example.org>, Jan Kara <email@example.com>, linux-fsdevel <firstname.lastname@example.org>, Linux Kernel Mailing List <email@example.com> 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 <firstname.lastname@example.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.
next prev parent 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 \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.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 \ email@example.com 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