Re: [PATCH 4/5] dax: use sb_issue_zerout instead of calling dax_clear_sectors
From: "Verma, Vishal L" <vishal.l.verma@intel.com>
Date: 2016-03-28 20:01:29
Also in:
linux-fsdevel, linux-mm, linux-xfs, nvdimm
On Fri, 2016-03-25 at 14:20 -0700, Dan Williams wrote:
On Fri, Mar 25, 2016 at 2:03 PM, Verma, Vishal L [off-list ref] wrote:quoted
On Fri, 2016-03-25 at 11:47 -0700, Dan Williams wrote:quoted
On Thu, Mar 24, 2016 at 4:17 PM, Vishal Verma <vishal.l.verma@int el.c om> wrote:quoted
From: Matthew Wilcox <redacted> dax_clear_sectors() cannot handle poisoned blocks. These must be zeroed using the BIO interface instead. Convert ext2 and XFS to use only sb_issue_zerout(). Signed-off-by: Matthew Wilcox <redacted> [vishal: Also remove the dax_clear_sectors function entirely] Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> --- fs/dax.c | 32 -------------------------------- fs/ext2/inode.c | 7 +++---- fs/xfs/xfs_bmap_util.c | 9 --------- include/linux/dax.h | 1 - 4 files changed, 3 insertions(+), 46 deletions(-)diff --git a/fs/dax.c b/fs/dax.c index bb7e9f8..a30481e 100644 --- a/fs/dax.c +++ b/fs/dax.c@@ -78,38 +78,6 @@ struct page *read_dax_sector(structblock_device *bdev, sector_t n) return page; } -/* - * dax_clear_sectors() is called from within transaction context from XFS, - * and hence this means the stack from this point must follow GFP_NOFS - * semantics for all operations. - */ -int dax_clear_sectors(struct block_device *bdev, sector_t _sector, long _size) -{ - struct blk_dax_ctl dax = { - .sector = _sector, - .size = _size, - }; - - might_sleep(); - do { - long count, sz; - - count = dax_map_atomic(bdev, &dax); - if (count < 0) - return count; - sz = min_t(long, count, SZ_128K); - clear_pmem(dax.addr, sz); - dax.size -= sz; - dax.sector += sz / 512; - dax_unmap_atomic(bdev, &dax); - cond_resched(); - } while (dax.size); - - wmb_pmem(); - return 0; -} -EXPORT_SYMBOL_GPL(dax_clear_sectors);What about the other unwritten extent conversions in the dax path? Shouldn't those be converted to block-layer zero-outs as well?Could you point me to where these might be? I thought once we've converted all the zeroout type callers (by removing dax_clear_sectors), and fixed up dax_do_io to try a driver fallback, we've handled all the media error cases in dax..grep for usages of clear_pmem()... which I was hoping to eliminate after this change to push zeroing down to the driver.
Ok, so I looked at these, and it looks like the majority of callers of clear_pmem are from the fault path (either pmd or regular), and in those cases we should be 'protected', as we would have failed at a prior step (dax_map_atomic). The two cases that may not be well handled are the calls to dax_zero_page_range and dax_truncate_page which are called from file systems. I think we may need to do a fallback to the driver for those cases just like we do for dax_direct_io.. Thoughts?