Dan Williams <dan.j.willi...@intel.com> writes:
> dax_clear_blocks is currently performing a cond_resched() after every
> PAGE_SIZE memset. We need not check so frequently, for example md-raid
> only calls cond_resched() at stripe granularity. Also, in preparation
> for introducing a dax_map_atomic() operation that temporarily pins a dax
> mapping move the call to cond_resched() to the outer loop.
There's nothing wrong with the mechanics here, but why bother? I only
see 1 caller in the kernel, and that caller passes in
1<<inode->i_blkbits for the size (so 1 page or less). Did you plan to
add other callers? I don't see them in this particular patch set.
Again, I'm not taking issue with the patch, I'm just wondering what
motivated the change.
Thanks!
Jeff
>
> Reviewed-by: Jan Kara <j...@suse.com>
> Signed-off-by: Dan Williams <dan.j.willi...@intel.com>
> ---
> fs/dax.c | 27 ++++++++++++---------------
> 1 file changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/fs/dax.c b/fs/dax.c
> index 5dc33d788d50..f8e543839e5c 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -28,6 +28,7 @@
> #include <linux/sched.h>
> #include <linux/uio.h>
> #include <linux/vmstat.h>
> +#include <linux/sizes.h>
>
> int dax_clear_blocks(struct inode *inode, sector_t block, long size)
> {
> @@ -38,24 +39,20 @@ int dax_clear_blocks(struct inode *inode, sector_t block,
> long size)
> do {
> void __pmem *addr;
> unsigned long pfn;
> - long count;
> + long count, sz;
>
> - count = bdev_direct_access(bdev, sector, &addr, &pfn, size);
> + sz = min_t(long, size, SZ_1M);
> + count = bdev_direct_access(bdev, sector, &addr, &pfn, sz);
> if (count < 0)
> return count;
> - BUG_ON(size < count);
> - while (count > 0) {
> - unsigned pgsz = PAGE_SIZE - offset_in_page(addr);
> - if (pgsz > count)
> - pgsz = count;
> - clear_pmem(addr, pgsz);
> - addr += pgsz;
> - size -= pgsz;
> - count -= pgsz;
> - BUG_ON(pgsz & 511);
> - sector += pgsz / 512;
> - cond_resched();
> - }
> + if (count < sz)
> + sz = count;
> + clear_pmem(addr, sz);
> + addr += sz;
> + size -= sz;
> + BUG_ON(sz & 511);
> + sector += sz / 512;
> + cond_resched();
> } while (size);
>
> wmb_pmem();
>
> _______________________________________________
> Linux-nvdimm mailing list
> linux-nvd...@lists.01.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
