On Sat, Jun 6, 2015 at 4:58 AM, Matthew Wilcox <willy@xxxxxxxxxxxxxxx> wrote: > On Fri, Jun 05, 2015 at 05:19:24PM -0400, Dan Williams wrote: >> @@ -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. >> >> -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. -- To unsubscribe from this list: send the line "unsubscribe linux-arch" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html