Re: [PATCH 05/13] x86, dax: replace clear_pmem() with open coded memset + dax_ops->flush

From: Jan Kara
Date: Fri Jan 20 2017 - 05:46:05 EST


On Thu 19-01-17 19:50:39, Dan Williams wrote:
> The clear_pmem() helper simply combines a memset() plus a cache flush.
> Now that the flush routine is optionally provided by the dax device
> driver we can avoid unnecessary cache management on dax devices fronting
> volatile memory.
>
> With clear_pmem() gone we can follow on with a patch to make pmem cache
> management completely defined within the pmem driver.
...
> diff --git a/fs/dax.c b/fs/dax.c
> index 160024e403f6..8883ce4d391e 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -986,6 +986,7 @@ static bool dax_range_is_aligned(struct block_device *bdev,
> int __dax_zero_page_range(struct block_device *bdev, sector_t sector,
> unsigned int offset, unsigned int length)
> {
> + const struct dax_operations *dax_ops = to_dax_ops(bdev);
> struct blk_dax_ctl dax = {
> .sector = sector,
> .size = PAGE_SIZE,
> @@ -999,7 +1000,9 @@ int __dax_zero_page_range(struct block_device *bdev, sector_t sector,
> } else {
> if (dax_map_atomic(bdev, &dax) < 0)
> return PTR_ERR(dax.addr);
> - clear_pmem(dax.addr + offset, length);
> + memset(dax.addr + offset, 0, length);
> + if (dax_ops->flush)
> + dax_ops->flush(dax.addr + offset, length);
> dax_unmap_atomic(bdev, &dax);
> }
> return 0;

Shouldn't we rather have some callback in dax_ops for clearing memory?
If we had all accesses to persistent memory inside DAX code wrapped inside
appropriate device wrappers that can report errors, we can have proper
error handling for the case we hit MCE, can't we?

Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR