From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Yangtao Li <frank.li@vivo.com>
Cc: linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH v2 1/2] f2fs: fix to avoid potential memory corruption in __update_iostat_latency()
Date: Wed, 18 Jan 2023 17:49:25 -0800 [thread overview]
Message-ID: <Y8ihpXw81e2FH2um@google.com> (raw)
In-Reply-To: <20230116134755.40119-1-frank.li@vivo.com>
On 01/16, Yangtao Li wrote:
> Hi Chao,
>
> > Maybe it's betterr to merge these two check condition as below?
> >
> > if (iotype >= NR_PAGE_TYPE) {
> > f2fs_bug_on(sbi, iotype != META_FLUSH);
> > iotype = META;
> > }
>
> For normal , only META_FLUSH will be greater than NR_PAGE_TYPE,
> there is no problem with this logic.
>
> >
> > For CHECK_FS is off case, I guess it's fine to not return and just print
> > warning message for notice.
>
> But if there is an exception, we don't know the type we originally wanted to count.
> If they are all changed to meta, it is possible to get a wrong statistic. I think
> there is no need to make statistics on this kind of error scene. Just like in the
> next patch, if iostat_lat_type is wrong, we should return directly instead of changing
> the value beyond the range to WRITE_ASYNC_IO.
>
> So it's no need tp merge these two check condition?
I also prefer to do like Chao's comment. We don't need to expect such exception
at all.
And, it seems we just need to do like:
enum page_type iotype;
if (iotype == META_FLUSH) {
iotype = META;
} else if (iotype >= NR_PAGE_TYPE) {
f2fs_warn();
return;
}
>
> Thx,
> Yangtao
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
WARNING: multiple messages have this Message-ID (diff)
From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Yangtao Li <frank.li@vivo.com>
Cc: chao@kernel.org, linux-f2fs-devel@lists.sourceforge.net,
linux-kernel@vger.kernel.org
Subject: Re: [f2fs-dev] [PATCH v2 1/2] f2fs: fix to avoid potential memory corruption in __update_iostat_latency()
Date: Wed, 18 Jan 2023 17:49:25 -0800 [thread overview]
Message-ID: <Y8ihpXw81e2FH2um@google.com> (raw)
In-Reply-To: <20230116134755.40119-1-frank.li@vivo.com>
On 01/16, Yangtao Li wrote:
> Hi Chao,
>
> > Maybe it's betterr to merge these two check condition as below?
> >
> > if (iotype >= NR_PAGE_TYPE) {
> > f2fs_bug_on(sbi, iotype != META_FLUSH);
> > iotype = META;
> > }
>
> For normal , only META_FLUSH will be greater than NR_PAGE_TYPE,
> there is no problem with this logic.
>
> >
> > For CHECK_FS is off case, I guess it's fine to not return and just print
> > warning message for notice.
>
> But if there is an exception, we don't know the type we originally wanted to count.
> If they are all changed to meta, it is possible to get a wrong statistic. I think
> there is no need to make statistics on this kind of error scene. Just like in the
> next patch, if iostat_lat_type is wrong, we should return directly instead of changing
> the value beyond the range to WRITE_ASYNC_IO.
>
> So it's no need tp merge these two check condition?
I also prefer to do like Chao's comment. We don't need to expect such exception
at all.
And, it seems we just need to do like:
enum page_type iotype;
if (iotype == META_FLUSH) {
iotype = META;
} else if (iotype >= NR_PAGE_TYPE) {
f2fs_warn();
return;
}
>
> Thx,
> Yangtao
next prev parent reply other threads:[~2023-01-19 1:49 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-16 13:02 [f2fs-dev] [PATCH v2 1/2] f2fs: fix to avoid potential memory corruption in __update_iostat_latency() Yangtao Li via Linux-f2fs-devel
2023-01-16 13:02 ` Yangtao Li
2023-01-16 13:25 ` [f2fs-dev] " Chao Yu
2023-01-16 13:25 ` Chao Yu
2023-01-16 13:47 ` [f2fs-dev] " Yangtao Li via Linux-f2fs-devel
2023-01-16 13:47 ` Yangtao Li
2023-01-19 1:49 ` Jaegeuk Kim [this message]
2023-01-19 1:49 ` Jaegeuk Kim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y8ihpXw81e2FH2um@google.com \
--to=jaegeuk@kernel.org \
--cc=frank.li@vivo.com \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.