Re: [PATCH v2 2/2] staging: erofs: fix i_blocks calculation
On 2019/5/28 14:57, Dan Carpenter wrote: > On Tue, May 28, 2019 at 11:02:12AM +0800, Chao Yu wrote: >> On 2019/5/28 10:36, Gao Xiang wrote: >>> For compressed files, i_blocks should not be calculated >>> by using i_size. i_u.compressed_blocks is used instead. >>> >>> In addition, i_blocks was miscalculated for non-compressed >>> files previously, fix it as well. >>> >>> Signed-off-by: Gao Xiang >>> --- >>> change log v2: >>> - fix description in commit message >>> - fix to 'inode->i_blocks = nblks << LOG_SECTORS_PER_BLOCK' >>> >>> Thanks, >>> Gao Xiang >>> >>> drivers/staging/erofs/inode.c | 14 -- >>> 1 file changed, 12 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c >>> index 8da144943ed6..6e67e018784e 100644 >>> --- a/drivers/staging/erofs/inode.c >>> +++ b/drivers/staging/erofs/inode.c >>> @@ -20,6 +20,7 @@ static int read_inode(struct inode *inode, void *data) >>> struct erofs_vnode *vi = EROFS_V(inode); >>> struct erofs_inode_v1 *v1 = data; >>> const unsigned int advise = le16_to_cpu(v1->i_advise); >>> + erofs_blk_t nblks = 0; >>> >>> vi->data_mapping_mode = __inode_data_mapping(advise); >>> >>> @@ -60,6 +61,10 @@ static int read_inode(struct inode *inode, void *data) >>> le32_to_cpu(v2->i_ctime_nsec); >>> >>> inode->i_size = le64_to_cpu(v2->i_size); >>> + >>> + /* total blocks for compressed files */ >>> + if (vi->data_mapping_mode == EROFS_INODE_LAYOUT_COMPRESSION) >>> + nblks = v2->i_u.compressed_blocks; >> >> Xiang, >> >> It needs to use le32_to_cpu(). ;) >> > > I wonder it the kbuild bot is going to send an email about that... 0-day may do this a little later. > Hopefully these sorts of bugs get detected with Sparse CF=-D__CHECK_ENDIAN__ Thanks, Dan, let's use this sparse flag more frequently to avoid such issue. Thanks, > > regards, > dan carpenter > > . > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/2] staging: erofs: fix i_blocks calculation
Hi Dan, On 2019/5/28 14:57, Dan Carpenter wrote: > On Tue, May 28, 2019 at 11:02:12AM +0800, Chao Yu wrote: >> On 2019/5/28 10:36, Gao Xiang wrote: >>> For compressed files, i_blocks should not be calculated >>> by using i_size. i_u.compressed_blocks is used instead. >>> >>> In addition, i_blocks was miscalculated for non-compressed >>> files previously, fix it as well. >>> >>> Signed-off-by: Gao Xiang >>> --- >>> change log v2: >>> - fix description in commit message >>> - fix to 'inode->i_blocks = nblks << LOG_SECTORS_PER_BLOCK' >>> >>> Thanks, >>> Gao Xiang >>> >>> drivers/staging/erofs/inode.c | 14 -- >>> 1 file changed, 12 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c >>> index 8da144943ed6..6e67e018784e 100644 >>> --- a/drivers/staging/erofs/inode.c >>> +++ b/drivers/staging/erofs/inode.c >>> @@ -20,6 +20,7 @@ static int read_inode(struct inode *inode, void *data) >>> struct erofs_vnode *vi = EROFS_V(inode); >>> struct erofs_inode_v1 *v1 = data; >>> const unsigned int advise = le16_to_cpu(v1->i_advise); >>> + erofs_blk_t nblks = 0; >>> >>> vi->data_mapping_mode = __inode_data_mapping(advise); >>> >>> @@ -60,6 +61,10 @@ static int read_inode(struct inode *inode, void *data) >>> le32_to_cpu(v2->i_ctime_nsec); >>> >>> inode->i_size = le64_to_cpu(v2->i_size); >>> + >>> + /* total blocks for compressed files */ >>> + if (vi->data_mapping_mode == EROFS_INODE_LAYOUT_COMPRESSION) >>> + nblks = v2->i_u.compressed_blocks; >> >> Xiang, >> >> It needs to use le32_to_cpu(). ;) >> > > I wonder it the kbuild bot is going to send an email about that... Not yet, and v3 fixes it. I have no idea whether kbuild checks all version or just the latest version... > Hopefully these sorts of bugs get detected with Sparse CF=-D__CHECK_ENDIAN__ Yes, I missed this case by mistake. These two patches are small, I didn't do too many static checking expect for checkpatch.pl. v3 seems fine and I will take care later, Thanks for kindly suggestion. :) Thanks, Gao Xiang > > regards, > dan carpenter > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/2] staging: erofs: fix i_blocks calculation
On Tue, May 28, 2019 at 11:02:12AM +0800, Chao Yu wrote: > On 2019/5/28 10:36, Gao Xiang wrote: > > For compressed files, i_blocks should not be calculated > > by using i_size. i_u.compressed_blocks is used instead. > > > > In addition, i_blocks was miscalculated for non-compressed > > files previously, fix it as well. > > > > Signed-off-by: Gao Xiang > > --- > > change log v2: > > - fix description in commit message > > - fix to 'inode->i_blocks = nblks << LOG_SECTORS_PER_BLOCK' > > > > Thanks, > > Gao Xiang > > > > drivers/staging/erofs/inode.c | 14 -- > > 1 file changed, 12 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c > > index 8da144943ed6..6e67e018784e 100644 > > --- a/drivers/staging/erofs/inode.c > > +++ b/drivers/staging/erofs/inode.c > > @@ -20,6 +20,7 @@ static int read_inode(struct inode *inode, void *data) > > struct erofs_vnode *vi = EROFS_V(inode); > > struct erofs_inode_v1 *v1 = data; > > const unsigned int advise = le16_to_cpu(v1->i_advise); > > + erofs_blk_t nblks = 0; > > > > vi->data_mapping_mode = __inode_data_mapping(advise); > > > > @@ -60,6 +61,10 @@ static int read_inode(struct inode *inode, void *data) > > le32_to_cpu(v2->i_ctime_nsec); > > > > inode->i_size = le64_to_cpu(v2->i_size); > > + > > + /* total blocks for compressed files */ > > + if (vi->data_mapping_mode == EROFS_INODE_LAYOUT_COMPRESSION) > > + nblks = v2->i_u.compressed_blocks; > > Xiang, > > It needs to use le32_to_cpu(). ;) > I wonder it the kbuild bot is going to send an email about that... Hopefully these sorts of bugs get detected with Sparse CF=-D__CHECK_ENDIAN__ regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/2] staging: erofs: fix i_blocks calculation
On 2019/5/28 11:02, Chao Yu wrote: > On 2019/5/28 10:36, Gao Xiang wrote: >> For compressed files, i_blocks should not be calculated >> by using i_size. i_u.compressed_blocks is used instead. >> >> In addition, i_blocks was miscalculated for non-compressed >> files previously, fix it as well. >> >> Signed-off-by: Gao Xiang >> --- >> change log v2: >> - fix description in commit message >> - fix to 'inode->i_blocks = nblks << LOG_SECTORS_PER_BLOCK' >> >> Thanks, >> Gao Xiang >> >> drivers/staging/erofs/inode.c | 14 -- >> 1 file changed, 12 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c >> index 8da144943ed6..6e67e018784e 100644 >> --- a/drivers/staging/erofs/inode.c >> +++ b/drivers/staging/erofs/inode.c >> @@ -20,6 +20,7 @@ static int read_inode(struct inode *inode, void *data) >> struct erofs_vnode *vi = EROFS_V(inode); >> struct erofs_inode_v1 *v1 = data; >> const unsigned int advise = le16_to_cpu(v1->i_advise); >> +erofs_blk_t nblks = 0; >> >> vi->data_mapping_mode = __inode_data_mapping(advise); >> >> @@ -60,6 +61,10 @@ static int read_inode(struct inode *inode, void *data) >> le32_to_cpu(v2->i_ctime_nsec); >> >> inode->i_size = le64_to_cpu(v2->i_size); >> + >> +/* total blocks for compressed files */ >> +if (vi->data_mapping_mode == EROFS_INODE_LAYOUT_COMPRESSION) >> +nblks = v2->i_u.compressed_blocks; > > Xiang, > > It needs to use le32_to_cpu(). ;) Already fixed in v3... Sorry about that... Thanks, Gao Xiang > >> } else if (__inode_version(advise) == EROFS_INODE_LAYOUT_V1) { >> struct erofs_sb_info *sbi = EROFS_SB(inode->i_sb); >> >> @@ -90,6 +95,8 @@ static int read_inode(struct inode *inode, void *data) >> sbi->build_time_nsec; >> >> inode->i_size = le32_to_cpu(v1->i_size); >> +if (vi->data_mapping_mode == EROFS_INODE_LAYOUT_COMPRESSION) >> +nblks = v1->i_u.compressed_blocks; > > Ditto. > > Thanks, > >> } else { >> errln("unsupported on-disk inode version %u of nid %llu", >>__inode_version(advise), vi->nid); >> @@ -97,8 +104,11 @@ static int read_inode(struct inode *inode, void *data) >> return -EIO; >> } >> >> -/* measure inode.i_blocks as the generic filesystem */ >> -inode->i_blocks = ((inode->i_size - 1) >> 9) + 1; >> +if (!nblks) >> +/* measure inode.i_blocks as generic filesystems */ >> +inode->i_blocks = roundup(inode->i_size, EROFS_BLKSIZ) >> 9; >> +else >> +inode->i_blocks = nblks << LOG_SECTORS_PER_BLOCK; >> return 0; >> } >> >> ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/2] staging: erofs: fix i_blocks calculation
On 2019/5/28 10:36, Gao Xiang wrote: > For compressed files, i_blocks should not be calculated > by using i_size. i_u.compressed_blocks is used instead. > > In addition, i_blocks was miscalculated for non-compressed > files previously, fix it as well. > > Signed-off-by: Gao Xiang > --- > change log v2: > - fix description in commit message > - fix to 'inode->i_blocks = nblks << LOG_SECTORS_PER_BLOCK' > > Thanks, > Gao Xiang > > drivers/staging/erofs/inode.c | 14 -- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c > index 8da144943ed6..6e67e018784e 100644 > --- a/drivers/staging/erofs/inode.c > +++ b/drivers/staging/erofs/inode.c > @@ -20,6 +20,7 @@ static int read_inode(struct inode *inode, void *data) > struct erofs_vnode *vi = EROFS_V(inode); > struct erofs_inode_v1 *v1 = data; > const unsigned int advise = le16_to_cpu(v1->i_advise); > + erofs_blk_t nblks = 0; > > vi->data_mapping_mode = __inode_data_mapping(advise); > > @@ -60,6 +61,10 @@ static int read_inode(struct inode *inode, void *data) > le32_to_cpu(v2->i_ctime_nsec); > > inode->i_size = le64_to_cpu(v2->i_size); > + > + /* total blocks for compressed files */ > + if (vi->data_mapping_mode == EROFS_INODE_LAYOUT_COMPRESSION) > + nblks = v2->i_u.compressed_blocks; Xiang, It needs to use le32_to_cpu(). ;) > } else if (__inode_version(advise) == EROFS_INODE_LAYOUT_V1) { > struct erofs_sb_info *sbi = EROFS_SB(inode->i_sb); > > @@ -90,6 +95,8 @@ static int read_inode(struct inode *inode, void *data) > sbi->build_time_nsec; > > inode->i_size = le32_to_cpu(v1->i_size); > + if (vi->data_mapping_mode == EROFS_INODE_LAYOUT_COMPRESSION) > + nblks = v1->i_u.compressed_blocks; Ditto. Thanks, > } else { > errln("unsupported on-disk inode version %u of nid %llu", > __inode_version(advise), vi->nid); > @@ -97,8 +104,11 @@ static int read_inode(struct inode *inode, void *data) > return -EIO; > } > > - /* measure inode.i_blocks as the generic filesystem */ > - inode->i_blocks = ((inode->i_size - 1) >> 9) + 1; > + if (!nblks) > + /* measure inode.i_blocks as generic filesystems */ > + inode->i_blocks = roundup(inode->i_size, EROFS_BLKSIZ) >> 9; > + else > + inode->i_blocks = nblks << LOG_SECTORS_PER_BLOCK; > return 0; > } > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel