* [GIT PULL] dax fixes for 4.20-rc6
@ 2018-12-09 6:26 Williams, Dan J
2018-12-09 18:01 ` Linus Torvalds
2018-12-09 18:50 ` pr-tracker-bot
0 siblings, 2 replies; 7+ messages in thread
From: Williams, Dan J @ 2018-12-09 6:26 UTC (permalink / raw)
To: torvalds; +Cc: linux-kernel, linux-nvdimm
Hi Linus, please pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm tags/dax-fixes-4.20-rc6
...to receive the last of the known regression fixes and fallout from
the Xarray conversion of the filesystem-dax implementation. On the path
to debugging why the dax memory-failure injection test started failing
after the Xarray conversion a couple more fixes for the
dax_lock_mapping_entry(), now called dax_lock_page(), surfaced. Those
plus the bug that started the hunt are now addressed. These patches
have appeared in a -next release with no issues reported.
Note the touches to mm/memory-failure.c are just the conversion to the
new function signature for dax_lock_page().
---
The following changes since commit 2e6e902d185027f8e3cb8b7305238f7e35d6a436:
Linux 4.20-rc4 (2018-11-25 14:19:31 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm tags/dax-fixes-4.20-rc6
for you to fetch changes up to 27359fd6e5f3c5db8fe544b63238b6170e8806d8:
dax: Fix unlock mismatch with updated API (2018-12-04 21:32:00 -0800)
----------------------------------------------------------------
dax fixes 4.20-rc6
* Fix the Xarray conversion of fsdax to properly handle
dax_lock_mapping_entry() in the presense of pmd entries.
* Fix inode destruction racing a new lock request.
----------------------------------------------------------------
Matthew Wilcox (3):
dax: Check page->mapping isn't NULL
dax: Don't access a freed inode
dax: Fix unlock mismatch with updated API
fs/dax.c | 55 ++++++++++++++++++++++++++++++++++++-----------------
include/linux/dax.h | 14 ++++++++------
mm/memory-failure.c | 6 ++++--
3 files changed, 50 insertions(+), 25 deletions(-)
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL] dax fixes for 4.20-rc6
2018-12-09 6:26 [GIT PULL] dax fixes for 4.20-rc6 Williams, Dan J
@ 2018-12-09 18:01 ` Linus Torvalds
2018-12-09 18:26 ` Dan Williams
2018-12-09 18:50 ` pr-tracker-bot
1 sibling, 1 reply; 7+ messages in thread
From: Linus Torvalds @ 2018-12-09 18:01 UTC (permalink / raw)
To: dan.j.williams; +Cc: Linux List Kernel Mailing, linux-nvdimm
On Sat, Dec 8, 2018 at 10:26 PM Williams, Dan J
<dan.j.williams@intel.com> wrote:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm tags/dax-fixes-4.20-rc6
What's going on with the odd non-exclusive exclusive wait?
prepare_to_wait_exclusive(wq, &ewait.wait, TASK_UNINTERRUPTIBLE);
...
/*
* Entry lock waits are exclusive. Wake up the next waiter since
* we aren't sure we will acquire the entry lock and thus wake
* the next waiter up on unlock.
*/
if (waitqueue_active(wq))
__wake_up(wq, TASK_NORMAL, 1, &ewait.key);
that seems to make little or no sense.
Why isn't that prepare_to_wait_exclusive() just a regular
prepare_to_wait(), and then the whole "let's wake up anybody else" can
be removed?
I've pulled it, but am awaiting explanation of what looks like some
pretty crazy code. I *suspect* it's a copy-and-paste situation where
you took the exclusive wait from somewhere else.
Linus
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL] dax fixes for 4.20-rc6
2018-12-09 18:01 ` Linus Torvalds
@ 2018-12-09 18:26 ` Dan Williams
2018-12-09 18:35 ` Linus Torvalds
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Dan Williams @ 2018-12-09 18:26 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jan Kara, Linux Kernel Mailing List, Matthew Wilcox, linux-nvdimm
[ add Willy and Jan ]
On Sun, Dec 9, 2018 at 10:02 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Sat, Dec 8, 2018 at 10:26 PM Williams, Dan J
> <dan.j.williams@intel.com> wrote:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm tags/dax-fixes-4.20-rc6
>
> What's going on with the odd non-exclusive exclusive wait?
>
> prepare_to_wait_exclusive(wq, &ewait.wait, TASK_UNINTERRUPTIBLE);
> ...
> /*
> * Entry lock waits are exclusive. Wake up the next waiter since
> * we aren't sure we will acquire the entry lock and thus wake
> * the next waiter up on unlock.
> */
> if (waitqueue_active(wq))
> __wake_up(wq, TASK_NORMAL, 1, &ewait.key);
>
> that seems to make little or no sense.
>
> Why isn't that prepare_to_wait_exclusive() just a regular
> prepare_to_wait(), and then the whole "let's wake up anybody else" can
> be removed?
>
> I've pulled it, but am awaiting explanation of what looks like some
> pretty crazy code. I *suspect* it's a copy-and-paste situation where
> you took the exclusive wait from somewhere else.
Yes, I believe that's true. In the other instances of waiting for an
entry to be in unlocked there is a guarantee that the waiter will
attain the lock and perform an unlock + wakeup. In the dax_lock_page()
path there is the possibility that the inode dies before the lock is
attained and a subsequent unlock sequence is not guaranteed. So, I
believe the intent, Willy correct me if I am wrong, was to keep all
waits "exclusive" for some sense of symmetry, but this one can and
should be a non-exclusive wait.
I can send a cleanup, do you want one immediately, or is post -rc6 ok?
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL] dax fixes for 4.20-rc6
2018-12-09 18:26 ` Dan Williams
@ 2018-12-09 18:35 ` Linus Torvalds
2018-12-09 19:49 ` Matthew Wilcox
2018-12-10 12:21 ` Jan Kara
2 siblings, 0 replies; 7+ messages in thread
From: Linus Torvalds @ 2018-12-09 18:35 UTC (permalink / raw)
To: dan.j.williams
Cc: Jan Kara, Linux List Kernel Mailing, Matthew Wilcox, linux-nvdimm
On Sun, Dec 9, 2018 at 10:27 AM Dan Williams <dan.j.williams@intel.com> wrote:
>
> I can send a cleanup, do you want one immediately, or is post -rc6 ok?
post-rc6 is fine, I'd rather have the patch tested anyway,
Linus
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL] dax fixes for 4.20-rc6
2018-12-09 6:26 [GIT PULL] dax fixes for 4.20-rc6 Williams, Dan J
2018-12-09 18:01 ` Linus Torvalds
@ 2018-12-09 18:50 ` pr-tracker-bot
1 sibling, 0 replies; 7+ messages in thread
From: pr-tracker-bot @ 2018-12-09 18:50 UTC (permalink / raw)
To: Williams, Dan J; +Cc: torvalds, linux-kernel, linux-nvdimm
The pull request you sent on Sun, 9 Dec 2018 06:26:39 +0000:
> git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm tags/dax-fixes-4.20-rc6
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/fa82dcbf2aed65dc3ea78eaca9ea56fd926b2b10
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL] dax fixes for 4.20-rc6
2018-12-09 18:26 ` Dan Williams
2018-12-09 18:35 ` Linus Torvalds
@ 2018-12-09 19:49 ` Matthew Wilcox
2018-12-10 12:21 ` Jan Kara
2 siblings, 0 replies; 7+ messages in thread
From: Matthew Wilcox @ 2018-12-09 19:49 UTC (permalink / raw)
To: Dan Williams
Cc: Jan Kara, Linus Torvalds, Linux Kernel Mailing List, linux-nvdimm
On Sun, Dec 09, 2018 at 10:26:54AM -0800, Dan Williams wrote:
> [ add Willy and Jan ]
>
> On Sun, Dec 9, 2018 at 10:02 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > On Sat, Dec 8, 2018 at 10:26 PM Williams, Dan J
> > <dan.j.williams@intel.com> wrote:
> > >
> > > git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm tags/dax-fixes-4.20-rc6
> >
> > What's going on with the odd non-exclusive exclusive wait?
> >
> > prepare_to_wait_exclusive(wq, &ewait.wait, TASK_UNINTERRUPTIBLE);
> > ...
> > /*
> > * Entry lock waits are exclusive. Wake up the next waiter since
> > * we aren't sure we will acquire the entry lock and thus wake
> > * the next waiter up on unlock.
> > */
> > if (waitqueue_active(wq))
> > __wake_up(wq, TASK_NORMAL, 1, &ewait.key);
> >
> > that seems to make little or no sense.
> >
> > Why isn't that prepare_to_wait_exclusive() just a regular
> > prepare_to_wait(), and then the whole "let's wake up anybody else" can
> > be removed?
> >
> > I've pulled it, but am awaiting explanation of what looks like some
> > pretty crazy code. I *suspect* it's a copy-and-paste situation where
> > you took the exclusive wait from somewhere else.
>
> Yes, I believe that's true. In the other instances of waiting for an
> entry to be in unlocked there is a guarantee that the waiter will
> attain the lock and perform an unlock + wakeup. In the dax_lock_page()
> path there is the possibility that the inode dies before the lock is
> attained and a subsequent unlock sequence is not guaranteed. So, I
> believe the intent, Willy correct me if I am wrong, was to keep all
> waits "exclusive" for some sense of symmetry, but this one can and
> should be a non-exclusive wait.
I did indeed just copy it from elsewhere. As I said at the time,
"This is the best I've come up with. Could we do better by not using
the _exclusive form of prepare_to_wait()? I'm not familiar with all the
things that need to be considered when using this family of interfaces."
but nobody suggested what the right way to use these interfaces was.
https://lists.01.org/pipermail/linux-nvdimm/2018-November/018892.html
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL] dax fixes for 4.20-rc6
2018-12-09 18:26 ` Dan Williams
2018-12-09 18:35 ` Linus Torvalds
2018-12-09 19:49 ` Matthew Wilcox
@ 2018-12-10 12:21 ` Jan Kara
2 siblings, 0 replies; 7+ messages in thread
From: Jan Kara @ 2018-12-10 12:21 UTC (permalink / raw)
To: Dan Williams
Cc: Jan Kara, Linus Torvalds, Linux Kernel Mailing List,
Matthew Wilcox, linux-nvdimm
On Sun 09-12-18 10:26:54, Dan Williams wrote:
> [ add Willy and Jan ]
>
> On Sun, Dec 9, 2018 at 10:02 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > On Sat, Dec 8, 2018 at 10:26 PM Williams, Dan J
> > <dan.j.williams@intel.com> wrote:
> > >
> > > git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm tags/dax-fixes-4.20-rc6
> >
> > What's going on with the odd non-exclusive exclusive wait?
> >
> > prepare_to_wait_exclusive(wq, &ewait.wait, TASK_UNINTERRUPTIBLE);
> > ...
> > /*
> > * Entry lock waits are exclusive. Wake up the next waiter since
> > * we aren't sure we will acquire the entry lock and thus wake
> > * the next waiter up on unlock.
> > */
> > if (waitqueue_active(wq))
> > __wake_up(wq, TASK_NORMAL, 1, &ewait.key);
> >
> > that seems to make little or no sense.
> >
> > Why isn't that prepare_to_wait_exclusive() just a regular
> > prepare_to_wait(), and then the whole "let's wake up anybody else" can
> > be removed?
> >
> > I've pulled it, but am awaiting explanation of what looks like some
> > pretty crazy code. I *suspect* it's a copy-and-paste situation where
> > you took the exclusive wait from somewhere else.
>
> Yes, I believe that's true. In the other instances of waiting for an
> entry to be in unlocked there is a guarantee that the waiter will
> attain the lock and perform an unlock + wakeup. In the dax_lock_page()
> path there is the possibility that the inode dies before the lock is
> attained and a subsequent unlock sequence is not guaranteed. So, I
> believe the intent, Willy correct me if I am wrong, was to keep all
> waits "exclusive" for some sense of symmetry, but this one can and
> should be a non-exclusive wait.
Agreed. I didn't realize this when reviewing Matthew's patch and
misunderstood his comment to this end.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-12-10 12:21 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-09 6:26 [GIT PULL] dax fixes for 4.20-rc6 Williams, Dan J
2018-12-09 18:01 ` Linus Torvalds
2018-12-09 18:26 ` Dan Williams
2018-12-09 18:35 ` Linus Torvalds
2018-12-09 19:49 ` Matthew Wilcox
2018-12-10 12:21 ` Jan Kara
2018-12-09 18:50 ` pr-tracker-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).