| Message ID | 20190301191241.27842-1-willy@infradead.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | b1b2bc60ca958dd0f2f89d833b0817a3a62b083c |
| Headers | show |
| Series | dax: Flush partial PMDs correctly | expand |
On Fri, Mar 1, 2019 at 11:12 AM Matthew Wilcox <willy@infradead.org> wrote: > > The radix tree would rewind the index in an iterator to the lowest index > of a multi-slot entry. The XArray iterators instead leave the index > unchanged, but I overlooked that when converting DAX from the radix tree > to the XArray. Adjust the index that we use for flushing to the start > of the PMD range. > > Fixes: c1901cd33cf4 "page cache: Convert find_get_entries_tag to XArray" > Reported-by: Piotr Balcer <piotr.balcer@intel.com> > Tested-by: Dan Williams <dan.j.williams@intel.com> Given this is a new patch my previous tested-by is invalid. ...that said, this one works. Tested-by: Dan Williams <dan.j.williams@intel.com>
On Fri 01-03-19 11:12:41, Matthew Wilcox wrote: > The radix tree would rewind the index in an iterator to the lowest index > of a multi-slot entry. The XArray iterators instead leave the index > unchanged, but I overlooked that when converting DAX from the radix tree > to the XArray. Adjust the index that we use for flushing to the start > of the PMD range. > > Fixes: c1901cd33cf4 "page cache: Convert find_get_entries_tag to XArray" > Reported-by: Piotr Balcer <piotr.balcer@intel.com> > Tested-by: Dan Williams <dan.j.williams@intel.com> > Signed-off-by: Matthew Wilcox <willy@infradead.org> > --- ... > @@ -917,8 +917,7 @@ static int dax_writeback_one(struct xa_state *xas, struct dax_device *dax_dev, > xas_clear_mark(xas, PAGECACHE_TAG_DIRTY); > dax_wake_entry(xas, entry, false); > > - trace_dax_writeback_one(mapping->host, xas->xa_index, > - size >> PAGE_SHIFT); > + trace_dax_writeback_one(mapping->host, xas->xa_index, count); I think here should be 'index' as well, shouldn't it? Otherwise the tracing data will be somewhat misleading... Besides this the patch looks good to me now so feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza
On Mon, Mar 11, 2019 at 2:06 AM Jan Kara <jack@suse.cz> wrote: > > On Fri 01-03-19 11:12:41, Matthew Wilcox wrote: > > The radix tree would rewind the index in an iterator to the lowest index > > of a multi-slot entry. The XArray iterators instead leave the index > > unchanged, but I overlooked that when converting DAX from the radix tree > > to the XArray. Adjust the index that we use for flushing to the start > > of the PMD range. > > > > Fixes: c1901cd33cf4 "page cache: Convert find_get_entries_tag to XArray" > > Reported-by: Piotr Balcer <piotr.balcer@intel.com> > > Tested-by: Dan Williams <dan.j.williams@intel.com> > > Signed-off-by: Matthew Wilcox <willy@infradead.org> > > --- > ... > > @@ -917,8 +917,7 @@ static int dax_writeback_one(struct xa_state *xas, struct dax_device *dax_dev, > > xas_clear_mark(xas, PAGECACHE_TAG_DIRTY); > > dax_wake_entry(xas, entry, false); > > > > - trace_dax_writeback_one(mapping->host, xas->xa_index, > > - size >> PAGE_SHIFT); > > + trace_dax_writeback_one(mapping->host, xas->xa_index, count); > > I think here should be 'index' as well, shouldn't it? Otherwise the tracing > data will be somewhat misleading... Besides this the patch looks good to me > now so feel free to add: > > Reviewed-by: Jan Kara <jack@suse.cz> I agree on the 'index' change, I'll fix that up. Thanks!
diff --git a/fs/dax.c b/fs/dax.c index 6959837cc465..4116cd9f55dd 100644 --- a/fs/dax.c +++ b/fs/dax.c @@ -843,9 +843,8 @@ static void dax_entry_mkclean(struct address_space *mapping, pgoff_t index, static int dax_writeback_one(struct xa_state *xas, struct dax_device *dax_dev, struct address_space *mapping, void *entry) { - unsigned long pfn; + unsigned long pfn, index, count; long ret = 0; - size_t size; /* * A page got tagged dirty in DAX mapping? Something is seriously @@ -894,17 +893,18 @@ static int dax_writeback_one(struct xa_state *xas, struct dax_device *dax_dev, xas_unlock_irq(xas); /* - * Even if dax_writeback_mapping_range() was given a wbc->range_start - * in the middle of a PMD, the 'index' we are given will be aligned to - * the start index of the PMD, as will the pfn we pull from 'entry'. + * If dax_writeback_mapping_range() was given a wbc->range_start + * in the middle of a PMD, the 'index' we use needs to be + * aligned to the start of the PMD. * This allows us to flush for PMD_SIZE and not have to worry about * partial PMD writebacks. */ pfn = dax_to_pfn(entry); - size = PAGE_SIZE << dax_entry_order(entry); + count = 1UL << dax_entry_order(entry); + index = xas->xa_index & ~(count - 1); - dax_entry_mkclean(mapping, xas->xa_index, pfn); - dax_flush(dax_dev, page_address(pfn_to_page(pfn)), size); + dax_entry_mkclean(mapping, index, pfn); + dax_flush(dax_dev, page_address(pfn_to_page(pfn)), count * PAGE_SIZE); /* * After we have flushed the cache, we can clear the dirty tag. There * cannot be new dirty data in the pfn after the flush has completed as @@ -917,8 +917,7 @@ static int dax_writeback_one(struct xa_state *xas, struct dax_device *dax_dev, xas_clear_mark(xas, PAGECACHE_TAG_DIRTY); dax_wake_entry(xas, entry, false); - trace_dax_writeback_one(mapping->host, xas->xa_index, - size >> PAGE_SHIFT); + trace_dax_writeback_one(mapping->host, xas->xa_index, count); return ret; put_unlocked: