* [PATCH V3 0/2] erofs: refactor fast_symlink and read_inode @ 2024-09-02 8:04 Yiyang Wu 2024-09-02 8:04 ` [PATCH V3 1/2] erofs: use kmemdup_nul in erofs_fill_symlink Yiyang Wu 2024-09-02 8:04 ` [PATCH V3 2/2] erofs: refactor read_inode calling convention Yiyang Wu 0 siblings, 2 replies; 6+ messages in thread From: Yiyang Wu @ 2024-09-02 8:04 UTC (permalink / raw) To: hsiangkao; +Cc: linux-erofs, linux-kernel, Yiyang Wu This patchset is response to the original suggestions posted here[1]. Since my later work is somehow related to this issue, i think it's better to deal with it first. Changes in V2: 1. Lift the erofs_fill_symlink patch to 1/2 2. Fix code styles problems in read_inode patch. 3. Fix the formatting problems caused by clang-format. Changes in V3: 1. Remove some redundant variables and statements in both patches. [1]: https://lore.kernel.org/all/20240425222847.GN2118490@ZenIV/ Yiyang Wu (2): erofs: use kmemdup_nul in erofs_fill_symlink erofs: refactor read_inode calling convention fs/erofs/inode.c | 126 ++++++++++++++++++++++------------------------- 1 file changed, 58 insertions(+), 68 deletions(-) -- 2.46.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH V3 1/2] erofs: use kmemdup_nul in erofs_fill_symlink 2024-09-02 8:04 [PATCH V3 0/2] erofs: refactor fast_symlink and read_inode Yiyang Wu @ 2024-09-02 8:04 ` Yiyang Wu 2024-09-02 8:08 ` Gao Xiang 2024-09-02 8:04 ` [PATCH V3 2/2] erofs: refactor read_inode calling convention Yiyang Wu 1 sibling, 1 reply; 6+ messages in thread From: Yiyang Wu @ 2024-09-02 8:04 UTC (permalink / raw) To: hsiangkao; +Cc: linux-erofs, linux-kernel, Yiyang Wu, Al Viro Remove open coding in erofs_fill_symlink. Suggested-by: Al Viro <viro@zeniv.linux.org.uk> Link: https://lore.kernel.org/all/20240425222847.GN2118490@ZenIV Signed-off-by: Yiyang Wu <toolmanp@tlmp.cc> --- fs/erofs/inode.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c index 419432be3223..40d3f4921d81 100644 --- a/fs/erofs/inode.c +++ b/fs/erofs/inode.c @@ -179,7 +179,6 @@ static int erofs_fill_symlink(struct inode *inode, void *kaddr, { struct erofs_inode *vi = EROFS_I(inode); unsigned int bsz = i_blocksize(inode); - char *lnk; /* if it cannot be handled with fast symlink scheme */ if (vi->datalayout != EROFS_INODE_FLAT_INLINE || @@ -188,24 +187,19 @@ static int erofs_fill_symlink(struct inode *inode, void *kaddr, return 0; } - lnk = kmalloc(inode->i_size + 1, GFP_KERNEL); - if (!lnk) - return -ENOMEM; - m_pofs += vi->xattr_isize; /* inline symlink data shouldn't cross block boundary */ if (m_pofs + inode->i_size > bsz) { - kfree(lnk); - erofs_err(inode->i_sb, - "inline data cross block boundary @ nid %llu", + erofs_err(inode->i_sb, "inline data cross block boundary @ nid %llu", vi->nid); DBG_BUGON(1); return -EFSCORRUPTED; } - memcpy(lnk, kaddr + m_pofs, inode->i_size); - lnk[inode->i_size] = '\0'; + + inode->i_link = kmemdup_nul(kaddr + m_pofs, inode->i_size, GFP_KERNEL); + if (!inode->i_link) + return -ENOMEM; - inode->i_link = lnk; inode->i_op = &erofs_fast_symlink_iops; return 0; } -- 2.46.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V3 1/2] erofs: use kmemdup_nul in erofs_fill_symlink 2024-09-02 8:04 ` [PATCH V3 1/2] erofs: use kmemdup_nul in erofs_fill_symlink Yiyang Wu @ 2024-09-02 8:08 ` Gao Xiang 0 siblings, 0 replies; 6+ messages in thread From: Gao Xiang @ 2024-09-02 8:08 UTC (permalink / raw) To: Yiyang Wu; +Cc: linux-erofs, linux-kernel, Al Viro On 2024/9/2 16:04, Yiyang Wu wrote: > Remove open coding in erofs_fill_symlink. > > Suggested-by: Al Viro <viro@zeniv.linux.org.uk> > Link: https://lore.kernel.org/all/20240425222847.GN2118490@ZenIV > Signed-off-by: Yiyang Wu <toolmanp@tlmp.cc> Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com> Thanks, Gao Xiang ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH V3 2/2] erofs: refactor read_inode calling convention 2024-09-02 8:04 [PATCH V3 0/2] erofs: refactor fast_symlink and read_inode Yiyang Wu 2024-09-02 8:04 ` [PATCH V3 1/2] erofs: use kmemdup_nul in erofs_fill_symlink Yiyang Wu @ 2024-09-02 8:04 ` Yiyang Wu 2024-09-02 8:12 ` Gao Xiang 1 sibling, 1 reply; 6+ messages in thread From: Yiyang Wu @ 2024-09-02 8:04 UTC (permalink / raw) To: hsiangkao; +Cc: linux-erofs, linux-kernel, Yiyang Wu, Al Viro Refactor out the iop binding behavior out of the erofs_fill_symlink and move erofs_buf into the erofs_read_inode, so that erofs_fill_inode can only deal with inode operation bindings and can be decoupled from metabuf operations. This results in better calling conventions. Note that after this patch, we do not need erofs_buf and ofs as parameters any more when calling erofs_read_inode as all the data operations are now included in itself. Suggested-by: Al Viro <viro@zeniv.linux.org.uk> Link: https://lore.kernel.org/all/20240425222847.GN2118490@ZenIV/ Signed-off-by: Yiyang Wu <toolmanp@tlmp.cc> --- fs/erofs/inode.c | 120 +++++++++++++++++++++++------------------------ 1 file changed, 58 insertions(+), 62 deletions(-) diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c index 40d3f4921d81..e9712af9a730 100644 --- a/fs/erofs/inode.c +++ b/fs/erofs/inode.c @@ -8,8 +8,33 @@ #include <trace/events/erofs.h> -static void *erofs_read_inode(struct erofs_buf *buf, - struct inode *inode, unsigned int *ofs) +static int erofs_fill_symlink(struct inode *inode, void *kaddr, + unsigned int m_pofs) +{ + struct erofs_inode *vi = EROFS_I(inode); + unsigned int bsz = i_blocksize(inode); + + /* if it cannot be handled with fast symlink scheme */ + if (vi->datalayout != EROFS_INODE_FLAT_INLINE || + inode->i_size >= bsz || inode->i_size < 0) { + return 0; + } + + m_pofs += vi->xattr_isize; + /* inline symlink data shouldn't cross block boundary */ + if (m_pofs + inode->i_size > bsz) { + erofs_err(inode->i_sb, "inline data cross block boundary @ nid %llu", + vi->nid); + DBG_BUGON(1); + return -EFSCORRUPTED; + } + + inode->i_link = kmemdup_nul(kaddr + m_pofs, inode->i_size, GFP_KERNEL); + + return inode->i_link ? 0 : -ENOMEM; +} + +static int erofs_read_inode(struct inode *inode) { struct super_block *sb = inode->i_sb; struct erofs_sb_info *sbi = EROFS_SB(sb); @@ -20,20 +45,21 @@ static void *erofs_read_inode(struct erofs_buf *buf, struct erofs_inode_compact *dic; struct erofs_inode_extended *die, *copied = NULL; union erofs_inode_i_u iu; - unsigned int ifmt; - int err; + struct erofs_buf buf; + unsigned int ifmt, ofs; + int err = 0; blkaddr = erofs_blknr(sb, inode_loc); - *ofs = erofs_blkoff(sb, inode_loc); + ofs = erofs_blkoff(sb, inode_loc); - kaddr = erofs_read_metabuf(buf, sb, erofs_pos(sb, blkaddr), EROFS_KMAP); + kaddr = erofs_read_metabuf(&buf, sb, erofs_pos(sb, blkaddr), EROFS_KMAP); if (IS_ERR(kaddr)) { erofs_err(sb, "failed to get inode (nid: %llu) page, err %ld", vi->nid, PTR_ERR(kaddr)); - return kaddr; + return PTR_ERR(kaddr); } - dic = kaddr + *ofs; + dic = kaddr + ofs; ifmt = le16_to_cpu(dic->i_format); if (ifmt & ~EROFS_I_ALL) { erofs_err(sb, "unsupported i_format %u of nid %llu", @@ -54,11 +80,11 @@ static void *erofs_read_inode(struct erofs_buf *buf, case EROFS_INODE_LAYOUT_EXTENDED: vi->inode_isize = sizeof(struct erofs_inode_extended); /* check if the extended inode acrosses block boundary */ - if (*ofs + vi->inode_isize <= sb->s_blocksize) { - *ofs += vi->inode_isize; + if (ofs + vi->inode_isize <= sb->s_blocksize) { + ofs += vi->inode_isize; die = (struct erofs_inode_extended *)dic; } else { - const unsigned int gotten = sb->s_blocksize - *ofs; + const unsigned int gotten = sb->s_blocksize - ofs; copied = kmalloc(vi->inode_isize, GFP_KERNEL); if (!copied) { @@ -66,16 +92,16 @@ static void *erofs_read_inode(struct erofs_buf *buf, goto err_out; } memcpy(copied, dic, gotten); - kaddr = erofs_read_metabuf(buf, sb, erofs_pos(sb, blkaddr + 1), + kaddr = erofs_read_metabuf(&buf, sb, erofs_pos(sb, blkaddr + 1), EROFS_KMAP); if (IS_ERR(kaddr)) { erofs_err(sb, "failed to get inode payload block (nid: %llu), err %ld", vi->nid, PTR_ERR(kaddr)); kfree(copied); - return kaddr; + return PTR_ERR(kaddr); } - *ofs = vi->inode_isize - gotten; - memcpy((u8 *)copied + gotten, kaddr, *ofs); + ofs = vi->inode_isize - gotten; + memcpy((u8 *)copied + gotten, kaddr, ofs); die = copied; } vi->xattr_isize = erofs_xattr_ibody_size(die->i_xattr_icount); @@ -91,11 +117,10 @@ static void *erofs_read_inode(struct erofs_buf *buf, inode->i_size = le64_to_cpu(die->i_size); kfree(copied); - copied = NULL; break; case EROFS_INODE_LAYOUT_COMPACT: vi->inode_isize = sizeof(struct erofs_inode_compact); - *ofs += vi->inode_isize; + ofs += vi->inode_isize; vi->xattr_isize = erofs_xattr_ibody_size(dic->i_xattr_icount); inode->i_mode = le16_to_cpu(dic->i_mode); @@ -119,6 +144,11 @@ static void *erofs_read_inode(struct erofs_buf *buf, case S_IFREG: case S_IFDIR: case S_IFLNK: + if(S_ISLNK(inode->i_mode)) { + err = erofs_fill_symlink(inode, kaddr, ofs); + if (err) + goto err_out; + } vi->raw_blkaddr = le32_to_cpu(iu.raw_blkaddr); break; case S_IFCHR: @@ -165,59 +195,24 @@ static void *erofs_read_inode(struct erofs_buf *buf, inode->i_blocks = round_up(inode->i_size, sb->s_blocksize) >> 9; else inode->i_blocks = nblks << (sb->s_blocksize_bits - 9); - return kaddr; err_out: - DBG_BUGON(1); - kfree(copied); - erofs_put_metabuf(buf); - return ERR_PTR(err); -} - -static int erofs_fill_symlink(struct inode *inode, void *kaddr, - unsigned int m_pofs) -{ - struct erofs_inode *vi = EROFS_I(inode); - unsigned int bsz = i_blocksize(inode); - - /* if it cannot be handled with fast symlink scheme */ - if (vi->datalayout != EROFS_INODE_FLAT_INLINE || - inode->i_size >= bsz || inode->i_size < 0) { - inode->i_op = &erofs_symlink_iops; - return 0; - } - - m_pofs += vi->xattr_isize; - /* inline symlink data shouldn't cross block boundary */ - if (m_pofs + inode->i_size > bsz) { - erofs_err(inode->i_sb, "inline data cross block boundary @ nid %llu", - vi->nid); - DBG_BUGON(1); - return -EFSCORRUPTED; - } - - inode->i_link = kmemdup_nul(kaddr + m_pofs, inode->i_size, GFP_KERNEL); - if (!inode->i_link) - return -ENOMEM; - - inode->i_op = &erofs_fast_symlink_iops; - return 0; + DBG_BUGON(err); + erofs_put_metabuf(&buf); + return err; } static int erofs_fill_inode(struct inode *inode) { struct erofs_inode *vi = EROFS_I(inode); - struct erofs_buf buf = __EROFS_BUF_INITIALIZER; - void *kaddr; - unsigned int ofs; int err = 0; trace_erofs_fill_inode(inode); /* read inode base data from disk */ - kaddr = erofs_read_inode(&buf, inode, &ofs); - if (IS_ERR(kaddr)) - return PTR_ERR(kaddr); + err = erofs_read_inode(inode); + if (err) + return err; /* setup the new inode */ switch (inode->i_mode & S_IFMT) { @@ -234,9 +229,11 @@ static int erofs_fill_inode(struct inode *inode) inode_nohighmem(inode); break; case S_IFLNK: - err = erofs_fill_symlink(inode, kaddr, ofs); - if (err) - goto out_unlock; + if (inode->i_link) + inode->i_op = &erofs_fast_symlink_iops; + else + inode->i_op = &erofs_symlink_iops; + inode_nohighmem(inode); break; case S_IFCHR: @@ -269,7 +266,6 @@ static int erofs_fill_inode(struct inode *inode) #endif } out_unlock: - erofs_put_metabuf(&buf); return err; } -- 2.46.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V3 2/2] erofs: refactor read_inode calling convention 2024-09-02 8:04 ` [PATCH V3 2/2] erofs: refactor read_inode calling convention Yiyang Wu @ 2024-09-02 8:12 ` Gao Xiang 2024-09-02 8:25 ` Yiyang Wu 0 siblings, 1 reply; 6+ messages in thread From: Gao Xiang @ 2024-09-02 8:12 UTC (permalink / raw) To: Yiyang Wu; +Cc: linux-erofs, linux-kernel, Al Viro On 2024/9/2 16:04, Yiyang Wu wrote: > Refactor out the iop binding behavior out of the erofs_fill_symlink > and move erofs_buf into the erofs_read_inode, so that erofs_fill_inode > can only deal with inode operation bindings and can be decoupled from > metabuf operations. This results in better calling conventions. > > Note that after this patch, we do not need erofs_buf and ofs as > parameters any more when calling erofs_read_inode as > all the data operations are now included in itself. > > Suggested-by: Al Viro <viro@zeniv.linux.org.uk> > Link: https://lore.kernel.org/all/20240425222847.GN2118490@ZenIV/ > Signed-off-by: Yiyang Wu <toolmanp@tlmp.cc> > --- > fs/erofs/inode.c | 120 +++++++++++++++++++++++------------------------ > 1 file changed, 58 insertions(+), 62 deletions(-) > > diff --git a/fs/erofs/inode.c b/fs/erofs/inode.c > index 40d3f4921d81..e9712af9a730 100644 > --- a/fs/erofs/inode.c > +++ b/fs/erofs/inode.c > @@ -8,8 +8,33 @@ > > #include <trace/events/erofs.h> > > -static void *erofs_read_inode(struct erofs_buf *buf, > - struct inode *inode, unsigned int *ofs) > +static int erofs_fill_symlink(struct inode *inode, void *kaddr, > + unsigned int m_pofs) > +{ > + struct erofs_inode *vi = EROFS_I(inode); > + unsigned int bsz = i_blocksize(inode); > + > + /* if it cannot be handled with fast symlink scheme */ > + if (vi->datalayout != EROFS_INODE_FLAT_INLINE || > + inode->i_size >= bsz || inode->i_size < 0) { > + return 0; > + } > + > + m_pofs += vi->xattr_isize; > + /* inline symlink data shouldn't cross block boundary */ > + if (m_pofs + inode->i_size > bsz) { > + erofs_err(inode->i_sb, "inline data cross block boundary @ nid %llu", > + vi->nid); > + DBG_BUGON(1); > + return -EFSCORRUPTED; > + } > + > + inode->i_link = kmemdup_nul(kaddr + m_pofs, inode->i_size, GFP_KERNEL); > + > + return inode->i_link ? 0 : -ENOMEM; > +} > + > +static int erofs_read_inode(struct inode *inode) > { > struct super_block *sb = inode->i_sb; > struct erofs_sb_info *sbi = EROFS_SB(sb); > @@ -20,20 +45,21 @@ static void *erofs_read_inode(struct erofs_buf *buf, > struct erofs_inode_compact *dic; > struct erofs_inode_extended *die, *copied = NULL; > union erofs_inode_i_u iu; > - unsigned int ifmt; > - int err; > + struct erofs_buf buf; Should be struct erofs_buf buf = __EROFS_BUF_INITIALIZER; Otherwise buf itself will be left uninitialized. > + unsigned int ifmt, ofs; > + int err = 0; > > blkaddr = erofs_blknr(sb, inode_loc); > - *ofs = erofs_blkoff(sb, inode_loc); > + ofs = erofs_blkoff(sb, inode_loc); > > - kaddr = erofs_read_metabuf(buf, sb, erofs_pos(sb, blkaddr), EROFS_KMAP); > + kaddr = erofs_read_metabuf(&buf, sb, erofs_pos(sb, blkaddr), EROFS_KMAP); > if (IS_ERR(kaddr)) { > erofs_err(sb, "failed to get inode (nid: %llu) page, err %ld", > vi->nid, PTR_ERR(kaddr)); > - return kaddr; > + return PTR_ERR(kaddr); > } > > - dic = kaddr + *ofs; > + dic = kaddr + ofs; > ifmt = le16_to_cpu(dic->i_format); > if (ifmt & ~EROFS_I_ALL) { > erofs_err(sb, "unsupported i_format %u of nid %llu", > @@ -54,11 +80,11 @@ static void *erofs_read_inode(struct erofs_buf *buf, > case EROFS_INODE_LAYOUT_EXTENDED: > vi->inode_isize = sizeof(struct erofs_inode_extended); > /* check if the extended inode acrosses block boundary */ > - if (*ofs + vi->inode_isize <= sb->s_blocksize) { > - *ofs += vi->inode_isize; > + if (ofs + vi->inode_isize <= sb->s_blocksize) { > + ofs += vi->inode_isize; > die = (struct erofs_inode_extended *)dic; > } else { > - const unsigned int gotten = sb->s_blocksize - *ofs; > + const unsigned int gotten = sb->s_blocksize - ofs; > > copied = kmalloc(vi->inode_isize, GFP_KERNEL); > if (!copied) { > @@ -66,16 +92,16 @@ static void *erofs_read_inode(struct erofs_buf *buf, > goto err_out; > } > memcpy(copied, dic, gotten); > - kaddr = erofs_read_metabuf(buf, sb, erofs_pos(sb, blkaddr + 1), > + kaddr = erofs_read_metabuf(&buf, sb, erofs_pos(sb, blkaddr + 1), > EROFS_KMAP); > if (IS_ERR(kaddr)) { > erofs_err(sb, "failed to get inode payload block (nid: %llu), err %ld", > vi->nid, PTR_ERR(kaddr)); > kfree(copied); > - return kaddr; > + return PTR_ERR(kaddr); > } > - *ofs = vi->inode_isize - gotten; > - memcpy((u8 *)copied + gotten, kaddr, *ofs); > + ofs = vi->inode_isize - gotten; > + memcpy((u8 *)copied + gotten, kaddr, ofs); > die = copied; > } > vi->xattr_isize = erofs_xattr_ibody_size(die->i_xattr_icount); > @@ -91,11 +117,10 @@ static void *erofs_read_inode(struct erofs_buf *buf, > > inode->i_size = le64_to_cpu(die->i_size); > kfree(copied); > - copied = NULL; > break; > case EROFS_INODE_LAYOUT_COMPACT: > vi->inode_isize = sizeof(struct erofs_inode_compact); > - *ofs += vi->inode_isize; > + ofs += vi->inode_isize; > vi->xattr_isize = erofs_xattr_ibody_size(dic->i_xattr_icount); > > inode->i_mode = le16_to_cpu(dic->i_mode); > @@ -119,6 +144,11 @@ static void *erofs_read_inode(struct erofs_buf *buf, > case S_IFREG: > case S_IFDIR: > case S_IFLNK: > + if(S_ISLNK(inode->i_mode)) { > + err = erofs_fill_symlink(inode, kaddr, ofs); > + if (err) > + goto err_out; > + } > vi->raw_blkaddr = le32_to_cpu(iu.raw_blkaddr); > break; > case S_IFCHR: > @@ -165,59 +195,24 @@ static void *erofs_read_inode(struct erofs_buf *buf, > inode->i_blocks = round_up(inode->i_size, sb->s_blocksize) >> 9; > else > inode->i_blocks = nblks << (sb->s_blocksize_bits - 9); > - return kaddr; > > err_out: > - DBG_BUGON(1); > - kfree(copied); > - erofs_put_metabuf(buf); > - return ERR_PTR(err); > -} > - > -static int erofs_fill_symlink(struct inode *inode, void *kaddr, > - unsigned int m_pofs) > -{ > - struct erofs_inode *vi = EROFS_I(inode); > - unsigned int bsz = i_blocksize(inode); > - > - /* if it cannot be handled with fast symlink scheme */ > - if (vi->datalayout != EROFS_INODE_FLAT_INLINE || > - inode->i_size >= bsz || inode->i_size < 0) { > - inode->i_op = &erofs_symlink_iops; > - return 0; > - } > - > - m_pofs += vi->xattr_isize; > - /* inline symlink data shouldn't cross block boundary */ > - if (m_pofs + inode->i_size > bsz) { > - erofs_err(inode->i_sb, "inline data cross block boundary @ nid %llu", > - vi->nid); > - DBG_BUGON(1); > - return -EFSCORRUPTED; > - } > - > - inode->i_link = kmemdup_nul(kaddr + m_pofs, inode->i_size, GFP_KERNEL); > - if (!inode->i_link) > - return -ENOMEM; > - > - inode->i_op = &erofs_fast_symlink_iops; > - return 0; > + DBG_BUGON(err); > + erofs_put_metabuf(&buf); > + return err; > } > > static int erofs_fill_inode(struct inode *inode) > { > struct erofs_inode *vi = EROFS_I(inode); > - struct erofs_buf buf = __EROFS_BUF_INITIALIZER; > - void *kaddr; > - unsigned int ofs; > int err = 0; > > trace_erofs_fill_inode(inode); > > /* read inode base data from disk */ > - kaddr = erofs_read_inode(&buf, inode, &ofs); > - if (IS_ERR(kaddr)) > - return PTR_ERR(kaddr); > + err = erofs_read_inode(inode); > + if (err) > + return err; > > /* setup the new inode */ > switch (inode->i_mode & S_IFMT) { > @@ -234,9 +229,11 @@ static int erofs_fill_inode(struct inode *inode) > inode_nohighmem(inode); > break; > case S_IFLNK: > - err = erofs_fill_symlink(inode, kaddr, ofs); > - if (err) > - goto out_unlock; > + if (inode->i_link) > + inode->i_op = &erofs_fast_symlink_iops; > + else > + inode->i_op = &erofs_symlink_iops; > + > inode_nohighmem(inode); > break; > case S_IFCHR: > @@ -269,7 +266,6 @@ static int erofs_fill_inode(struct inode *inode) > #endif > } > out_unlock: I mean since `return err` is the only useful statement for label `out_unlock`. You could just remove it entirely and replace all `goto out_unlock` to `return err`; the name `out_unlock` itself is also meaningless due to this patch. Thanks, Gao Xiang > - erofs_put_metabuf(&buf); > return err; > } > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V3 2/2] erofs: refactor read_inode calling convention 2024-09-02 8:12 ` Gao Xiang @ 2024-09-02 8:25 ` Yiyang Wu 0 siblings, 0 replies; 6+ messages in thread From: Yiyang Wu @ 2024-09-02 8:25 UTC (permalink / raw) To: Gao Xiang; +Cc: linux-erofs, linux-kernel On Mon, Sep 02, 2024 at 04:12:31PM GMT, Gao Xiang wrote: > > > > +static int erofs_read_inode(struct inode *inode) > > { > > struct super_block *sb = inode->i_sb; > > struct erofs_sb_info *sbi = EROFS_SB(sb); > > @@ -20,20 +45,21 @@ static void *erofs_read_inode(struct erofs_buf *buf, > > struct erofs_inode_compact *dic; > > struct erofs_inode_extended *die, *copied = NULL; > > union erofs_inode_i_u iu; > > - unsigned int ifmt; > > - int err; > > + struct erofs_buf buf; > > Should be > struct erofs_buf buf = __EROFS_BUF_INITIALIZER; > > Otherwise buf itself will be left uninitialized. > My bad, i forget this when rebasing. will be fixed in next version. > > inode_nohighmem(inode); > > break; > > case S_IFCHR: > > @@ -269,7 +266,6 @@ static int erofs_fill_inode(struct inode *inode) > > #endif > > } > > out_unlock: > > I mean since `return err` is the only useful statement for > label `out_unlock`. > > You could just remove it entirely and replace all > `goto out_unlock` to `return err`; > > the name `out_unlock` itself is also meaningless due to > this patch. > > Thanks, > Gao Xiang > > > - erofs_put_metabuf(&buf); > > return err; > > } > Got it, will be fixed in next version. Best Regards, Yiyang Wu ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-09-02 8:26 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-09-02 8:04 [PATCH V3 0/2] erofs: refactor fast_symlink and read_inode Yiyang Wu 2024-09-02 8:04 ` [PATCH V3 1/2] erofs: use kmemdup_nul in erofs_fill_symlink Yiyang Wu 2024-09-02 8:08 ` Gao Xiang 2024-09-02 8:04 ` [PATCH V3 2/2] erofs: refactor read_inode calling convention Yiyang Wu 2024-09-02 8:12 ` Gao Xiang 2024-09-02 8:25 ` Yiyang Wu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®