Thread (41 messages) 41 messages, 4 authors, 2016-10-12

Re: [PATCH v5 09/17] dax: coordinate locking for offsets in PMD range

From: Ross Zwisler <hidden>
Date: 2016-10-11 21:18:04
Also in: linux-fsdevel, linux-mm, linux-xfs, lkml, nvdimm

On Tue, Oct 11, 2016 at 09:04:09AM +0200, Jan Kara wrote:
On Fri 07-10-16 15:08:56, Ross Zwisler wrote:
quoted
DAX radix tree locking currently locks entries based on the unique
combination of the 'mapping' pointer and the pgoff_t 'index' for the entry.
This works for PTEs, but as we move to PMDs we will need to have all the
offsets within the range covered by the PMD to map to the same bit lock.
To accomplish this, for ranges covered by a PMD entry we will instead lock
based on the page offset of the beginning of the PMD entry.  The 'mapping'
pointer is still used in the same way.

Signed-off-by: Ross Zwisler <redacted>
The patch looks good to me. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

Just one thing which IMO deserves a comment below:
quoted
@@ -448,9 +460,12 @@ restart:
 }
 
 void dax_wake_mapping_entry_waiter(struct address_space *mapping,
-				   pgoff_t index, bool wake_all)
+		pgoff_t index, void *entry, bool wake_all)
 {
-	wait_queue_head_t *wq = dax_entry_waitqueue(mapping, index);
+	struct exceptional_entry_key key;
+	wait_queue_head_t *wq;
+
+	wq = dax_entry_waitqueue(mapping, index, entry, &key);
So I believe we should comment above this function that the 'entry' it gets
may be invalid by the time it gets it (we call it without tree_lock held so
the passed entry may be changed in the radix tree as we work) but we use it
only to find appropriate waitqueue where tasks sleep waiting for that old
entry to unlock so we indeed wake up all tasks we need.
Added, thanks for the suggestion.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help