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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170120102717.GI14115@quack2.suse.cz>
Date:   Fri, 20 Jan 2017 11:27:17 +0100
From:   Jan Kara <jack@...e.cz>
To:     Dan Williams <dan.j.williams@...el.com>
Cc:     linux-nvdimm@...ts.01.org, Jan Kara <jack@...e.cz>,
        Matthew Wilcox <mawilcox@...rosoft.com>, x86@...nel.org,
        linux-kernel@...r.kernel.org, Christoph Hellwig <hch@....de>,
        Jeff Moyer <jmoyer@...hat.com>, Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, linux-fsdevel@...r.kernel.org,
        Thomas Gleixner <tglx@...utronix.de>,
        Ross Zwisler <ross.zwisler@...ux.intel.com>
Subject: Re: [PATCH 05/13] x86, dax: replace clear_pmem() with open coded
 memset + dax_ops->flush

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@...e.com>
SUSE Labs, CR