| Message ID | 1406252943-5834-1-git-send-email-quwenruo@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
Hi Qu, (2014/07/25 10:49), Qu Wenruo wrote: > When page aligned start and len passed to extent_fiemap(), the result is > good, but when start and len is not aligned, e.g. start = 1 and len = > 4095 is passed to extent_fiemap(), it returns no extent. > > The problem is that start and len is all rounded up which causes the > problem. This patch will round down start and round up (start + len) to > return right extent. > > Reported-by: Chandan Rajendra <chandan@linux.vnet.ibm.com> > Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> > --- > changelog: > v2: reword the description(ALIGN rounds up, not rounds down). > --- > fs/btrfs/extent_io.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c > index a389820..1c70cff 100644 > --- a/fs/btrfs/extent_io.c > +++ b/fs/btrfs/extent_io.c > @@ -4213,8 +4213,8 @@ int extent_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, > return -ENOMEM; > path->leave_spinning = 1; > > - start = ALIGN(start, BTRFS_I(inode)->root->sectorsize); > - len = ALIGN(len, BTRFS_I(inode)->root->sectorsize); > + start = round_down(start, BTRFS_I(inode)->root->sectorsize); > + len = round_up(max, BTRFS_I(inode)->root->sectorsize) - start; I have one question. Why isn't it "len = round_up(len, BTRFS_I(inode)->root->sectorsize);" ? Old behavior make len sectorsize aligned. However, if start is not aligned to sectorsize, new bahavior make len not sectorsize aligned. Does it an expected behavior? CMIIW. I'm not good at extent code. Thanks, Satoru > > /* > * lookup the last file extent. We're not using i_size here > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
-------- Original Message -------- Subject: Re: [PATCH v2] btrfs: Return right extent when fiemap gives unaligned offset and len. From: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com> To: Qu Wenruo <quwenruo@cn.fujitsu.com>, <linux-btrfs@vger.kernel.org> Date: 2014?07?25? 15:52 > Hi Qu, > > (2014/07/25 10:49), Qu Wenruo wrote: >> When page aligned start and len passed to extent_fiemap(), the result is >> good, but when start and len is not aligned, e.g. start = 1 and len = >> 4095 is passed to extent_fiemap(), it returns no extent. >> >> The problem is that start and len is all rounded up which causes the >> problem. This patch will round down start and round up (start + len) to >> return right extent. >> >> Reported-by: Chandan Rajendra <chandan@linux.vnet.ibm.com> >> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> >> --- >> changelog: >> v2: reword the description(ALIGN rounds up, not rounds down). >> --- >> fs/btrfs/extent_io.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c >> index a389820..1c70cff 100644 >> --- a/fs/btrfs/extent_io.c >> +++ b/fs/btrfs/extent_io.c >> @@ -4213,8 +4213,8 @@ int extent_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, >> return -ENOMEM; >> path->leave_spinning = 1; >> >> - start = ALIGN(start, BTRFS_I(inode)->root->sectorsize); >> - len = ALIGN(len, BTRFS_I(inode)->root->sectorsize); >> + start = round_down(start, BTRFS_I(inode)->root->sectorsize); >> + len = round_up(max, BTRFS_I(inode)->root->sectorsize) - start; > I have one question. > > Why isn't it "len = round_up(len, BTRFS_I(inode)->root->sectorsize);" ? > Old behavior make len sectorsize aligned. However, if start is not > aligned to sectorsize, new bahavior make len not sectorsize aligned. > Does it an expected behavior? In fact, in my code, len is also aligned. The calculation is somewhat easy, round_down original 'start' and round_up original end(in the code, max is original unaligned end position), then calculate 'len' by minus aligned end with aligned 'start'. Since the *new* 'start' is already aligned and the new end is aligned, the len will also be aligned. Thanks, Qu > > CMIIW. I'm not good at extent code. > > Thanks, > Satoru > >> >> /* >> * lookup the last file extent. We're not using i_size here >> -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Qu, (2014/07/25 17:16), Qu Wenruo wrote: > > -------- Original Message -------- > Subject: Re: [PATCH v2] btrfs: Return right extent when fiemap gives unaligned offset and len. > From: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com> > To: Qu Wenruo <quwenruo@cn.fujitsu.com>, <linux-btrfs@vger.kernel.org> > Date: 2014?07?25? 15:52 >> Hi Qu, >> >> (2014/07/25 10:49), Qu Wenruo wrote: >>> When page aligned start and len passed to extent_fiemap(), the result is >>> good, but when start and len is not aligned, e.g. start = 1 and len = >>> 4095 is passed to extent_fiemap(), it returns no extent. >>> >>> The problem is that start and len is all rounded up which causes the >>> problem. This patch will round down start and round up (start + len) to >>> return right extent. >>> >>> Reported-by: Chandan Rajendra <chandan@linux.vnet.ibm.com> >>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> >>> --- >>> changelog: >>> v2: reword the description(ALIGN rounds up, not rounds down). >>> --- >>> fs/btrfs/extent_io.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c >>> index a389820..1c70cff 100644 >>> --- a/fs/btrfs/extent_io.c >>> +++ b/fs/btrfs/extent_io.c >>> @@ -4213,8 +4213,8 @@ int extent_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, >>> return -ENOMEM; >>> path->leave_spinning = 1; >>> - start = ALIGN(start, BTRFS_I(inode)->root->sectorsize); >>> - len = ALIGN(len, BTRFS_I(inode)->root->sectorsize); >>> + start = round_down(start, BTRFS_I(inode)->root->sectorsize); >>> + len = round_up(max, BTRFS_I(inode)->root->sectorsize) - start; >> I have one question. >> >> Why isn't it "len = round_up(len, BTRFS_I(inode)->root->sectorsize);" ? >> Old behavior make len sectorsize aligned. However, if start is not >> aligned to sectorsize, new bahavior make len not sectorsize aligned. >> Does it an expected behavior? > In fact, in my code, len is also aligned. > The calculation is somewhat easy, round_down original 'start' and round_up original end(in the code, max is original > unaligned end position), > then calculate 'len' by minus aligned end with aligned 'start'. > Since the *new* 'start' is already aligned and the new end is aligned, the len will also be aligned. Thank you for your explanation. I understood. I forgot to that "start" is aligned here. Reviewed-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com> > > Thanks, > Qu >> >> CMIIW. I'm not good at extent code. >> >> Thanks, >> Satoru >> >>> /* >>> * lookup the last file extent. We're not using i_size here >>> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 25, 2014 at 09:49:03AM +0800, Qu Wenruo wrote: > When page aligned start and len passed to extent_fiemap(), the result is > good, but when start and len is not aligned, e.g. start = 1 and len = > 4095 is passed to extent_fiemap(), it returns no extent. > > The problem is that start and len is all rounded up which causes the > problem. This patch will round down start and round up (start + len) to > return right extent. > > Reported-by: Chandan Rajendra <chandan@linux.vnet.ibm.com> > Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> Reviewed-by: David Sterba <dsterba@suse.cz> > --- > changelog: > v2: reword the description(ALIGN rounds up, not rounds down). JFYI, if you're just rewording the changelog, you can keep the reviewed-by tags. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index a389820..1c70cff 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -4213,8 +4213,8 @@ int extent_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, return -ENOMEM; path->leave_spinning = 1; - start = ALIGN(start, BTRFS_I(inode)->root->sectorsize); - len = ALIGN(len, BTRFS_I(inode)->root->sectorsize); + start = round_down(start, BTRFS_I(inode)->root->sectorsize); + len = round_up(max, BTRFS_I(inode)->root->sectorsize) - start; /* * lookup the last file extent. We're not using i_size here
When page aligned start and len passed to extent_fiemap(), the result is good, but when start and len is not aligned, e.g. start = 1 and len = 4095 is passed to extent_fiemap(), it returns no extent. The problem is that start and len is all rounded up which causes the problem. This patch will round down start and round up (start + len) to return right extent. Reported-by: Chandan Rajendra <chandan@linux.vnet.ibm.com> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com> --- changelog: v2: reword the description(ALIGN rounds up, not rounds down). --- fs/btrfs/extent_io.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)