RE: regression introduced by "block: Add support for DAX reads/writes to block devices"

Wilcox, Matthew R
2015-08-07T18:11:32Z

So ... what you're doing here is if somebody requests the last 512 bytes, you're asking for the last page. That's going to work as long as the partition is a multiple of PAGE_SIZE, but not if the partition happens to be misaligned. It's an improvement, although I'd be tempted to do this as:

if (pos == max) {
unsigned blkbits = inode->i_blkbits;
- sector_t block = pos >> blkbits;
+ long page = pos >> PAGE_SHIFT;
+ sector_t block = page << (PAGE_SHIFT - blkbits);
unsigned first = pos - (block << blkbits);
long size;

We need to cope with the case where the end of a partition isn't on a page boundary though.

-----Original Message-----
From: Jeff Moyer [mailto:jmoyer@redhat.com]
Sent: Thursday, August 06, 2015 2:31 PM
To: Wilcox, Matthew R
Cc: linda.knippers@hp.com; linux-kernel@vger.kernel.org; linux-fsdevel@vger.kernel.org
Subject: Re: regression introduced by "block: Add support for DAX reads/writes to block devices"

"Wilcox, Matthew R" <matthew.r.wilcox@intel.com> writes:

> I think I see the problem. I'm kind of wrapped up in other things
> right now; can you try replacing the line in dax_io():
>
> - bh->b_size = PAGE_ALIGN(end - pos);
> + bh->b_size = ALIGN(end - pos, 1 << blkbits);

This works for me. If it looks okay to others, I'll submit a properly
signed-off patch.

Cheers,
Jeff

diff --git a/fs/dax.c b/fs/dax.c
index a7f77e1..b6c4f93 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -98,6 +98,10 @@ static bool buffer_size_valid(struct buffer_head *bh)
return bh->b_state != 0;
}

+/*
+ * This function assumes file system block size (represented by
+ * inode->i_blkbits) is less than or equal to the system page size.
+ */
static ssize_t dax_io(struct inode *inode, struct iov_iter *iter,
loff_t start, loff_t end, get_block_t get_block,
struct buffer_head *bh)
@@ -117,9 +121,15 @@ static ssize_t dax_io(struct inode *inode, struct iov_iter *iter,
if (pos == max) {
unsigned blkbits = inode->i_blkbits;
sector_t block = pos >> blkbits;
- unsigned first = pos - (block << blkbits);
+ long page = pos >> PAGE_SHIFT;
+ unsigned first; /* byte offset into block */
long size;

+ /* we can only map entire pages */
+ if (pos & (PAGE_SIZE-1))
+ block = page << (PAGE_SHIFT - blkbits);
+ first = pos - (block << blkbits);
+
if (pos == bh_max) {
bh->b_size = PAGE_ALIGN(end - pos);
bh->b_state = 0;