Re: [f2fs-dev] [PATCH] f2fs: Fix incorrect return value
On 2024/4/9 14:47, wangjianjian (C) via Linux-f2fs-devel wrote:
On 2024/4/7 14:23, Chao Yu wrote:
On 2024/4/4 21:47, Wang Jianjian wrote:
dquot_mark_dquot_dirty returns old dirty state not the error code.
I think it's fine to just pass return value of dquot_mark_dquot_dirty()
to caller, because caller can distinguish status from return value:
1) < 0, there is an error, 2) >= 0, there is no error, previously it is
dirty if it is 1.
mark_all_dquot_dirty uses if return value is 0 to save error code. It may cause
mess.
I didn't get your point...
No caller of mark_all_dquot_dirty() cares about its return value, so,
I think there is no practical problem now.
By the way, I am fine don't change it.
Thanks,
Signed-off-by: Wang Jianjian
---
fs/f2fs/super.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index a6867f26f141..af07027475d9 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -3063,13 +3063,13 @@ static int f2fs_dquot_mark_dquot_dirty(struct dquot
*dquot)
{
struct super_block *sb = dquot->dq_sb;
struct f2fs_sb_info *sbi = F2FS_SB(sb);
- int ret = dquot_mark_dquot_dirty(dquot);
+ dquot_mark_dquot_dirty(dquot);
/* if we are using journalled quota */
if (is_journalled_quota(sbi))
set_sbi_flag(sbi, SBI_QUOTA_NEED_FLUSH);
- return ret;
+ return 0;
}
static int f2fs_dquot_commit_info(struct super_block *sb, int type)
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: Fix incorrect return value
On 2024/4/7 14:23, Chao Yu wrote:
On 2024/4/4 21:47, Wang Jianjian wrote:
dquot_mark_dquot_dirty returns old dirty state not the error code.
I think it's fine to just pass return value of dquot_mark_dquot_dirty()
to caller, because caller can distinguish status from return value:
1) < 0, there is an error, 2) >= 0, there is no error, previously it is
dirty if it is 1.
mark_all_dquot_dirty uses if return value is 0 to save error code. It
may cause mess.
By the way, I am fine don't change it.
Thanks,
Signed-off-by: Wang Jianjian
---
fs/f2fs/super.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index a6867f26f141..af07027475d9 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -3063,13 +3063,13 @@ static int f2fs_dquot_mark_dquot_dirty(struct
dquot *dquot)
{
struct super_block *sb = dquot->dq_sb;
struct f2fs_sb_info *sbi = F2FS_SB(sb);
- int ret = dquot_mark_dquot_dirty(dquot);
+ dquot_mark_dquot_dirty(dquot);
/* if we are using journalled quota */
if (is_journalled_quota(sbi))
set_sbi_flag(sbi, SBI_QUOTA_NEED_FLUSH);
- return ret;
+ return 0;
}
static int f2fs_dquot_commit_info(struct super_block *sb, int type)
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
--
Regards
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: Fix incorrect return value
On 2024/4/4 21:47, Wang Jianjian wrote:
dquot_mark_dquot_dirty returns old dirty state not the error code.
I think it's fine to just pass return value of dquot_mark_dquot_dirty()
to caller, because caller can distinguish status from return value:
1) < 0, there is an error, 2) >= 0, there is no error, previously it is
dirty if it is 1.
Thanks,
Signed-off-by: Wang Jianjian
---
fs/f2fs/super.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index a6867f26f141..af07027475d9 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -3063,13 +3063,13 @@ static int f2fs_dquot_mark_dquot_dirty(struct dquot
*dquot)
{
struct super_block *sb = dquot->dq_sb;
struct f2fs_sb_info *sbi = F2FS_SB(sb);
- int ret = dquot_mark_dquot_dirty(dquot);
+ dquot_mark_dquot_dirty(dquot);
/* if we are using journalled quota */
if (is_journalled_quota(sbi))
set_sbi_flag(sbi, SBI_QUOTA_NEED_FLUSH);
- return ret;
+ return 0;
}
static int f2fs_dquot_commit_info(struct super_block *sb, int type)
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] f2fs: Fix incorrect return value
dquot_mark_dquot_dirty returns old dirty state not the error code.
Signed-off-by: Wang Jianjian
---
fs/f2fs/super.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index a6867f26f141..af07027475d9 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -3063,13 +3063,13 @@ static int f2fs_dquot_mark_dquot_dirty(struct dquot
*dquot)
{
struct super_block *sb = dquot->dq_sb;
struct f2fs_sb_info *sbi = F2FS_SB(sb);
- int ret = dquot_mark_dquot_dirty(dquot);
+ dquot_mark_dquot_dirty(dquot);
/* if we are using journalled quota */
if (is_journalled_quota(sbi))
set_sbi_flag(sbi, SBI_QUOTA_NEED_FLUSH);
- return ret;
+ return 0;
}
static int f2fs_dquot_commit_info(struct super_block *sb, int type)
--
2.34.3
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: fix incorrect return value in f2fs_sanity_check_ckpt()
Could you post the patch again? I don't see this in my box.
On 10/27, Chao Yu wrote:
> Jaegeuk,
>
> Missed to apply this patch?
>
> Thanks,
>
> On 2021/9/24 17:50, Pavel Machek wrote:
> > Hi!
> >
> > > This code looks quite confused: part of function returns 1 on
> > > corruption, part returns -errno. The problem is not stable-specific.
> > >
> > > [1] https://lkml.org/lkml/2021/9/19/207
> > >
> > > Let's fix to make 'insane cp_payload case' to return 1 rater than
> > > EFSCORRUPTED, so that return value can be kept consistent for all
> > > error cases, it can avoid confusion of code logic.
> > >
> > > Fixes: 65ddf6564843 ("f2fs: fix to do sanity check for sb/cp fields
> > > correctly")
> > > Reported-by: Pavel Machek
> > > Signed-off-by: Chao Yu
> >
> > Reviewed-by: Pavel Machek
> >
> > (This is good minimal fix, but eventually I believe the function
> > should switch to 0/-errno... for consistency with rest of kernel).
> >
> > Thank you,
> > Pavel
> >
> > > +++ b/fs/f2fs/super.c
> > > @@ -3487,7 +3487,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> > > NR_CURSEG_PERSIST_TYPE + nat_bits_blocks >=
> > > blocks_per_seg)) {
> > > f2fs_warn(sbi, "Insane cp_payload: %u, nat_bits_blocks:
> > > %u)",
> > > cp_payload, nat_bits_blocks);
> > > - return -EFSCORRUPTED;
> > > + return 1;
> > > }
> > > if (unlikely(f2fs_cp_error(sbi))) {
> >
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: fix incorrect return value in f2fs_sanity_check_ckpt()
Jaegeuk,
Missed to apply this patch?
Thanks,
On 2021/9/24 17:50, Pavel Machek wrote:
Hi!
This code looks quite confused: part of function returns 1 on
corruption, part returns -errno. The problem is not stable-specific.
[1] https://lkml.org/lkml/2021/9/19/207
Let's fix to make 'insane cp_payload case' to return 1 rater than
EFSCORRUPTED, so that return value can be kept consistent for all
error cases, it can avoid confusion of code logic.
Fixes: 65ddf6564843 ("f2fs: fix to do sanity check for sb/cp fields correctly")
Reported-by: Pavel Machek
Signed-off-by: Chao Yu
Reviewed-by: Pavel Machek
(This is good minimal fix, but eventually I believe the function
should switch to 0/-errno... for consistency with rest of kernel).
Thank you,
Pavel
+++ b/fs/f2fs/super.c
@@ -3487,7 +3487,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
NR_CURSEG_PERSIST_TYPE + nat_bits_blocks >= blocks_per_seg)) {
f2fs_warn(sbi, "Insane cp_payload: %u, nat_bits_blocks: %u)",
cp_payload, nat_bits_blocks);
- return -EFSCORRUPTED;
+ return 1;
}
if (unlikely(f2fs_cp_error(sbi))) {
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] f2fs: fix incorrect return value in f2fs_sanity_check_ckpt()
As Pavel Machek reported in [1]
This code looks quite confused: part of function returns 1 on
corruption, part returns -errno. The problem is not stable-specific.
[1] https://lkml.org/lkml/2021/9/19/207
Let's fix to make 'insane cp_payload case' to return 1 rater than
EFSCORRUPTED, so that return value can be kept consistent for all
error cases, it can avoid confusion of code logic.
Fixes: 65ddf6564843 ("f2fs: fix to do sanity check for sb/cp fields correctly")
Reported-by: Pavel Machek
Signed-off-by: Chao Yu
---
fs/f2fs/super.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 8d6d0657a470..e3975cb8e3e8 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -3487,7 +3487,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
NR_CURSEG_PERSIST_TYPE + nat_bits_blocks >= blocks_per_seg)) {
f2fs_warn(sbi, "Insane cp_payload: %u, nat_bits_blocks: %u)",
cp_payload, nat_bits_blocks);
- return -EFSCORRUPTED;
+ return 1;
}
if (unlikely(f2fs_cp_error(sbi))) {
--
2.32.0
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
