Re: [PATCH v6 4/5] dax: for truncate/hole-punch, do zeroing through the driver if possible

Jan Kara
2016-05-11T08:15:32Z

On Tue 10-05-16 12:49:15, Vishal Verma wrote:
> In the truncate or hole-punch path in dax, we clear out sub-page ranges.
> If these sub-page ranges are sector aligned and sized, we can do the
> zeroing through the driver instead so that error-clearing is handled
> automatically.
>
> For sub-sector ranges, we still have to rely on clear_pmem and have the
> possibility of tripping over errors.
>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Ross Zwisler <ross.zwisler@linux.intel.com>
> Cc: Jeff Moyer <jmoyer@redhat.com>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Dave Chinner <david@fromorbit.com>
> Cc: Jan Kara <jack@suse.cz>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>

...

> +static bool dax_range_is_aligned(struct block_device *bdev,
> + struct blk_dax_ctl *dax, unsigned int offset,
> + unsigned int length)
> +{
> + unsigned short sector_size = bdev_logical_block_size(bdev);
> +
> + if (!IS_ALIGNED(((u64)dax->addr + offset), sector_size))

One more question: 'dax' is initialized in dax_zero_page_range() and
dax->addr is going to be always NULL here. So either you forgot to call
dax_map_atomic() to get the addr or the use of dax->addr is just bogus
(which is what I currently believe since I see no way how the address could
be unaligned with the sector_size)...

Honza
> + return false;
> + if (!IS_ALIGNED(length, sector_size))
> + return false;
> +
> + return true;
> +}
> +
> /**
> * dax_zero_page_range - zero a range within a page of a DAX file
> * @inode: The file being truncated
> @@ -1240,11 +1254,16 @@ int dax_zero_page_range(struct inode *inode, loff_t from, unsigned length,
> .size = PAGE_SIZE,
> };
>
> - if (dax_map_atomic(bdev, &dax) < 0)
> - return PTR_ERR(dax.addr);
> - clear_pmem(dax.addr + offset, length);
> - wmb_pmem();
> - dax_unmap_atomic(bdev, &dax);
> + if (dax_range_is_aligned(bdev, &dax, offset, length))
> + return blkdev_issue_zeroout(bdev, dax.sector,
> + length >> 9, GFP_NOFS, true);
> + else {
> + if (dax_map_atomic(bdev, &dax) < 0)
> + return PTR_ERR(dax.addr);
> + clear_pmem(dax.addr + offset, length);
> + wmb_pmem();
> + dax_unmap_atomic(bdev, &dax);
> + }
> }
>
> return 0;
> --
> 2.5.5
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR