Re: [Linux-nvdimm] [PATCH 1/3] pmem: Initial version of persistent memory driver

[Christoph Hellwig <hch@lst.de>]

On Thu, Mar 26, 2015 at 07:12:23AM -0700, Dan Williams wrote:
> > +       struct resource *res_mem;
> > +       int err;
> > +
> > +       res_mem = request_mem_region_exclusive(pmem->phys_addr, pmem->size,
> > +                                              "pmem");
> 
> Isn't request_mem_region() enough?  i.e. it seems
> request_mem_region_exclusive() assumes no DAX, at least in theory?

This is 1:1 from the patch Ross sent, but I've been wondering why
request_mem_region_exclusive is used here.  All it does is setting the
IORESOURCE_EXCLUSIVE flag, which prevents /dev/mem and sysfs from accessing
the memory while the driver claims it. Besides pmem only a watchdog driver
and e1000 make use of this flag, and there's various function related to
it that are entirely unused.  It's a weird beast.

> This is fine for now, but I think we're going to end up with a
> continuum of solutions to this problem based on the platform and the
> device.  Some ADR platforms have firmware that takes actions like
> flushing caches on a "power going away" signal.  Other platforms have
> cache management instructions that we can use on either a per-i/o or
> per REQ_FUA/FLUSH request.  Hmm, with this being in the memory map by
> default I think this poses a challenge for VIVT caches and aliased
> accesses?  We can revisit this when arm support shows up.

Yes, I expect us to pass flags related to this through the platform_data
eventually, but I think that starting with the simplest and safest version
is probably the best idea.