| Message ID | 1462571591-3361-4-git-send-email-vishal.l.verma@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, May 06, 2016 at 03:53:09PM -0600, Vishal Verma wrote: > From: Matthew Wilcox <matthew.r.wilcox@intel.com> > > 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 <matthew.r.wilcox@intel.com> > [vishal: Also remove the dax_clear_sectors function entirely] > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> Just to make sure: the existing sb_issue_zerout as in 4.6-rc is already doing the right thing for DAX? I've got a pending patchset for XFS that introduces another dax_clear_sectors users, but if it's already safe to use blkdev_issue_zeroout I can switch to that and avoid the merge conflict. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 2016-05-08 at 01:52 -0700, Christoph Hellwig wrote: > On Fri, May 06, 2016 at 03:53:09PM -0600, Vishal Verma wrote: > > > > From: Matthew Wilcox <matthew.r.wilcox@intel.com> > > > > 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 <matthew.r.wilcox@intel.com> > > [vishal: Also remove the dax_clear_sectors function entirely] > > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > Just to make sure: the existing sb_issue_zerout as in 4.6-rc > is already doing the right thing for DAX? I've got a pending > patchset > for XFS that introduces another dax_clear_sectors users, but if it's > already safe to use blkdev_issue_zeroout I can switch to that and > avoid > the merge conflict. I believe so - Jan has moved all unwritten extent conversions out of DAX with his patch set, and I believe zeroing through the driver is always fine. Ross or Jan could confirm though.
On Sun, May 08, 2016 at 06:46:13PM +0000, Verma, Vishal L wrote: > On Sun, 2016-05-08 at 01:52 -0700, Christoph Hellwig wrote: > > On Fri, May 06, 2016 at 03:53:09PM -0600, Vishal Verma wrote: > > > > > > From: Matthew Wilcox <matthew.r.wilcox@intel.com> > > > > > > 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 <matthew.r.wilcox@intel.com> > > > [vishal: Also remove the dax_clear_sectors function entirely] > > > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > > Just to make sure: the existing sb_issue_zerout as in 4.6-rc > > is already doing the right thing for DAX? I've got a pending > > patchset > > for XFS that introduces another dax_clear_sectors users, but if it's > > already safe to use blkdev_issue_zeroout I can switch to that and > > avoid > > the merge conflict. > > I believe so - Jan has moved all unwritten extent conversions out of > DAX with his patch set, and I believe zeroing through the driver is > always fine. Ross or Jan could confirm though. Yep, I believe that the existing sb_issue_zeroout() as of v4.6-rc* does the right thing. We'll end up calling sb_issue_zeroout() => blkdev_issue_zeroout() => __blkdev_issue_zeroout() because we don't have support for discard or write_same in PMEM. This will send zero page BIOs to the PMEM driver, which will do the zeroing as normal writes. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri 06-05-16 15:53:09, Vishal Verma wrote: > From: Matthew Wilcox <matthew.r.wilcox@intel.com> > > 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 <matthew.r.wilcox@intel.com> > [vishal: Also remove the dax_clear_sectors function entirely] > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> The patch looks good. You can add: Reviewed-by: Jan Kara <jack@suse.cz> Honza
diff --git a/fs/dax.c b/fs/dax.c index 52f0044..5948d9b 100644 --- a/fs/dax.c +++ b/fs/dax.c @@ -116,38 +116,6 @@ struct page *read_dax_sector(struct block_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); - static bool buffer_written(struct buffer_head *bh) { return buffer_mapped(bh) && !buffer_unwritten(bh); diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c index 1f07b75..35f2b0bf 100644 --- a/fs/ext2/inode.c +++ b/fs/ext2/inode.c @@ -26,6 +26,7 @@ #include <linux/highuid.h> #include <linux/pagemap.h> #include <linux/dax.h> +#include <linux/blkdev.h> #include <linux/quotaops.h> #include <linux/writeback.h> #include <linux/buffer_head.h> @@ -737,10 +738,8 @@ static int ext2_get_blocks(struct inode *inode, * so that it's not found by another thread before it's * initialised */ - err = dax_clear_sectors(inode->i_sb->s_bdev, - le32_to_cpu(chain[depth-1].key) << - (inode->i_blkbits - 9), - 1 << inode->i_blkbits); + err = sb_issue_zeroout(inode->i_sb, + le32_to_cpu(chain[depth-1].key), 1, GFP_NOFS); if (err) { mutex_unlock(&ei->truncate_mutex); goto cleanup; diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c index 3b63098..930ac6a 100644 --- a/fs/xfs/xfs_bmap_util.c +++ b/fs/xfs/xfs_bmap_util.c @@ -72,18 +72,11 @@ xfs_zero_extent( struct xfs_mount *mp = ip->i_mount; xfs_daddr_t sector = xfs_fsb_to_db(ip, start_fsb); sector_t block = XFS_BB_TO_FSBT(mp, sector); - ssize_t size = XFS_FSB_TO_B(mp, count_fsb); - - if (IS_DAX(VFS_I(ip))) - return dax_clear_sectors(xfs_find_bdev_for_inode(VFS_I(ip)), - sector, size); - - /* - * let the block layer decide on the fastest method of - * implementing the zeroing. - */ - return sb_issue_zeroout(mp->m_super, block, count_fsb, GFP_NOFS); + return blkdev_issue_zeroout(xfs_find_bdev_for_inode(VFS_I(ip)), + block << (mp->m_super->s_blocksize_bits - 9), + count_fsb << (mp->m_super->s_blocksize_bits - 9), + GFP_NOFS, true); } /* diff --git a/include/linux/dax.h b/include/linux/dax.h index ef94fa7..426841a 100644 --- a/include/linux/dax.h +++ b/include/linux/dax.h @@ -11,7 +11,6 @@ ssize_t dax_do_io(struct kiocb *, struct inode *, struct iov_iter *, loff_t, get_block_t, dio_iodone_t, int flags); -int dax_clear_sectors(struct block_device *bdev, sector_t _sector, long _size); int dax_zero_page_range(struct inode *, loff_t from, unsigned len, get_block_t); int dax_truncate_page(struct inode *, loff_t from, get_block_t); int dax_fault(struct vm_area_struct *, struct vm_fault *, get_block_t);