On Wed, Feb 1, 2017 at 12:10 AM, Christoph Hellwig <hch@xxxxxx> wrote: > On Mon, Jan 30, 2017 at 10:16:29AM -0800, Dan Williams wrote: >> Ok, now that dax_map_atomic() is gone, it's much easier to remove >> struct blk_dax_ctl. >> >> We can also move the partition alignment checks to be a one-time check >> at bdev_dax_capable() time and kill bdev_dax_direct_access() in favor >> of calling dax_direct_access() directly. > > Yes, please. > >> >> + if ((sector + DIV_ROUND_UP(dax->size, 512)) >> >> + > part_nr_sects_read(bdev->bd_part)) >> >> + return -ERANGE; >> >> + sector += get_start_sect(bdev); >> >> + return dax_direct_access(dax_inode, sector * 512, &dax->addr, >> >> + &dax->pfn, dax->size); >> > >> > And please switch to using bytes as the granularity given that we're >> > deadling with byte addressable memory. >> >> dax_direct_access() does take a byte aligned physical address, but it >> needs to be at least page aligned since we are returning a pfn_t... >> >> Hmm, perhaps the input should be raw page frame number. We could >> reduce one of the arguments by making the current 'pfn_t *' parameter >> an in/out-parameter. > > In/Out parameters are always a bit problematic in terms of API clarity. > And updating a device-relative address with an absolute physical one > sounds like an odd API for sure. Yes, it does, and I thought better of it shortly after sending that. How about: long dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff, unsigned long nr_pages, void **kaddr, pfn_t *pfn)
