Messages in this thread | | | Date | Sun, 9 Dec 2018 11:49:46 -0800 | From | Matthew Wilcox <> | Subject | Re: [GIT PULL] dax fixes for 4.20-rc6 |
| |
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
| |