Re: [PATCH v4 4/9] dax: fix mapping lifetime handling, convert to __pfn_t + kmap_atomic_pfn_t()
From: Dan Williams <hidden>
Date: 2015-08-07 23:54:07
Also in:
linux-fsdevel, lkml, nvdimm
On Sat, Jun 6, 2015 at 4:58 AM, Matthew Wilcox [off-list ref] wrote:
On Fri, Jun 05, 2015 at 05:19:24PM -0400, Dan Williams wrote:quoted
@@ -35,13 +35,16 @@ int dax_clear_blocks(struct inode *inode, sector_t block, long size) might_sleep(); do { void *addr; - unsigned long pfn; + __pfn_t pfn; long count; - count = bdev_direct_access(bdev, sector, &addr, &pfn, size); + count = bdev_direct_access(bdev, sector, &pfn, size); if (count < 0) return count; BUG_ON(size < count); + addr = kmap_atomic_pfn_t(pfn); + if (!addr) + return -EIO; while (count > 0) { unsigned pgsz = PAGE_SIZE - offset_in_page(addr); if (pgsz > count)This part is incomplete. When bdev_direct_access() could return an address, it was possible for that address to be unaligned (eg when 'sector' was not a multiple of 8). DAX has never had full support for devices that weren't a 4k sector size, but I was trying to not make that assumption in more places than I had to. So this function needs a lot more simplification (or it needs to add '(sector & 7) << 9' to addr ... assuming that the partition this bdev represents actually starts at a multiple of 8 ... bleh!).
Isn't this already handled by the:
if (sector % (PAGE_SIZE / 512))
return -EINVAL;
...check in bdev_direct_access()? As long as the driver's mapping is
4K aligned, which appears to be the case for all DAX-enabled drivers,
then we should be good to go.
quoted
-static long dax_get_addr(struct buffer_head *bh, void **addr, unsigned blkbits) +static long dax_get_pfn(struct buffer_head *bh, __pfn_t *pfn, unsigned blkbits) { - unsigned long pfn; sector_t sector = bh->b_blocknr << (blkbits - 9); - return bdev_direct_access(bh->b_bdev, sector, addr, &pfn, bh->b_size); + return bdev_direct_access(bh->b_bdev, sector, pfn, bh->b_size); }This function should just be deleted. It offers essentially nothing over just calling bdev_direct_access().
Ok.