| Message ID | 20200203200029.4592-5-vgoyal@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | dax,pmem: Provide a dax operation to zero range of memory | expand |
On Mon, Feb 03, 2020 at 03:00:28PM -0500, Vivek Goyal wrote: > + id = dax_read_lock(); > + rc = dax_zero_page_range(dax_dev, pgoff, offset, size); > + dax_read_unlock(id); > + return rc; Is there a good reason not to move the locking into dax_zero_page_range?
On Wed, Feb 05, 2020 at 10:33:56AM -0800, Christoph Hellwig wrote: > On Mon, Feb 03, 2020 at 03:00:28PM -0500, Vivek Goyal wrote: > > + id = dax_read_lock(); > > + rc = dax_zero_page_range(dax_dev, pgoff, offset, size); > > + dax_read_unlock(id); > > + return rc; > > Is there a good reason not to move the locking into dax_zero_page_range? No reason. I can move locking inside dax_zero_page_range(). Will do. Vivek
On Wed, Feb 05, 2020 at 10:33:56AM -0800, Christoph Hellwig wrote: > On Mon, Feb 03, 2020 at 03:00:28PM -0500, Vivek Goyal wrote: > > + id = dax_read_lock(); > > + rc = dax_zero_page_range(dax_dev, pgoff, offset, size); > > + dax_read_unlock(id); > > + return rc; > > Is there a good reason not to move the locking into dax_zero_page_range? Thinking more about it. If we keep locking outside, then we don't have to take lock again when we recurse into dax_zero_page_range() in device mapper path. IIUC, just taking lock once at top level is enough. If that's the case then it probably is better to keep locking outside of dax_zero_page_range(). Thanks Vivek
diff --git a/fs/dax.c b/fs/dax.c index 35631a4d0295..1b9ba6b59cdb 100644 --- a/fs/dax.c +++ b/fs/dax.c @@ -1044,19 +1044,6 @@ static vm_fault_t dax_load_hole(struct xa_state *xas, return ret; } -static bool dax_range_is_aligned(struct block_device *bdev, - unsigned int offset, unsigned int length) -{ - unsigned short sector_size = bdev_logical_block_size(bdev); - - if (!IS_ALIGNED(offset, sector_size)) - return false; - if (!IS_ALIGNED(length, sector_size)) - return false; - - return true; -} - int generic_dax_zero_page_range(struct dax_device *dax_dev, pgoff_t pgoff, unsigned int offset, size_t len) { @@ -1076,31 +1063,17 @@ int __dax_zero_page_range(struct block_device *bdev, struct dax_device *dax_dev, sector_t sector, unsigned int offset, unsigned int size) { - if (dax_range_is_aligned(bdev, offset, size)) { - sector_t start_sector = sector + (offset >> 9); - - return blkdev_issue_zeroout(bdev, start_sector, - size >> 9, GFP_NOFS, 0); - } else { - pgoff_t pgoff; - long rc, id; - void *kaddr; + pgoff_t pgoff; + long rc, id; - rc = bdev_dax_pgoff(bdev, sector, PAGE_SIZE, &pgoff); - if (rc) - return rc; + rc = bdev_dax_pgoff(bdev, sector, PAGE_SIZE, &pgoff); + if (rc) + return rc; - id = dax_read_lock(); - rc = dax_direct_access(dax_dev, pgoff, 1, &kaddr, NULL); - if (rc < 0) { - dax_read_unlock(id); - return rc; - } - memset(kaddr + offset, 0, size); - dax_flush(dax_dev, kaddr + offset, size); - dax_read_unlock(id); - } - return 0; + id = dax_read_lock(); + rc = dax_zero_page_range(dax_dev, pgoff, offset, size); + dax_read_unlock(id); + return rc; } EXPORT_SYMBOL_GPL(__dax_zero_page_range);
Get rid of calling block device interface for zeroing in iomap dax zeroing path and use dax native zeroing interface instead. Suggested-by: Christoph Hellwig <hch@infradead.org> Signed-off-by: Vivek Goyal <vgoyal@redhat.com> --- fs/dax.c | 45 +++++++++------------------------------------ 1 file changed, 9 insertions(+), 36 deletions(-)