On Sun, Nov 08, 2015 at 02:27:44PM -0500, Dan Williams wrote:
> +++ b/fs/block_dev.c
> @@ -464,7 +464,7 @@ long bdev_direct_access(struct block_device *bdev, sector_t sector,
> if (sector % (PAGE_SIZE / 512))
> return -EINVAL;
> avail = ops->direct_access(bdev, sector, addr, pfn);
> - if (!avail)
> + if (!avail || avail < PAGE_SIZE)
> return -ERANGE;
> return min(avail, size);
> }
... redundant? avail == 0 is covered by avail < PAGE_SIZE.
> diff --git a/fs/dax.c b/fs/dax.c
> index 149d6000d72a..b1ac67de0654 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -30,6 +30,40 @@
> #include <linux/vmstat.h>
> #include <linux/sizes.h>
>
> +static void __pmem *__dax_map_atomic(struct block_device *bdev, sector_t sector,
> + long size, unsigned long *pfn, long *len)
> +{
> + long rc;
> + void __pmem *addr;
> + struct request_queue *q = bdev->bd_queue;
> +
> + if (blk_queue_enter(q, GFP_NOWAIT) != 0)
> + return (void __pmem *) ERR_PTR(-EIO);
> + rc = bdev_direct_access(bdev, sector, &addr, pfn, size);
> + if (len)
> + *len = rc;
> + if (rc < 0) {
> + blk_queue_exit(q);
> + return (void __pmem *) ERR_PTR(rc);
> + }
> + return addr;
> +}
I don't like this helper function. You've changed the calling convention to
'return a pointer, which might be an error value' from the rest of the
DAX code which is using 'return a length; if it's negative that's an error'.
> @@ -42,9 +76,9 @@ int dax_clear_blocks(struct inode *inode, sector_t block, long size)
> long count, sz;
>
> sz = min_t(long, size, SZ_1M);
> - count = bdev_direct_access(bdev, sector, &addr, &pfn, sz);
> - if (count < 0)
> - return count;
> + addr = __dax_map_atomic(bdev, sector, size, &pfn, &count);
> + if (IS_ERR(addr))
> + return PTR_ERR(addr);
I don't have anything against the PTR_ERR/ERR_PTR convention per se,
but you've complicated the callers, plus there's now this awkward bit
in the middle of the code where the convention suddenly changes. Since
you need to return both the length and the address from the function,
one of them has to be passed by reference.
--
To unsubscribe from this list: send the line "unsubscribe linux-block" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
