Thread (30 messages) 30 messages, 6 authors, 2015-08-07

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help