[f2fs-dev] [PATCH v3] f2fs: fix using wrong 'submitted' value in f2fs_write_cache_pages

[f2fs-dev] [PATCH v3] f2fs: fix using wrong 'submitted' value in f2fs_write_cache_pages

[zangyangyang]

From: zangyangyang1 <zangyangyang1@xiaomi.com>

When f2fs_write_single_data_page fails, f2fs_write_cache_pages
will use the last 'submitted' value incorrectly, which will cause
'nwritten' and 'wbc->nr_to_write' calculation errors

Signed-off-by: zangyangyang1 <zangyangyang1@xiaomi.com>
---
v3: No logical changes, just format patch
v2: Initialize "submitted" in f2fs_write_single_data_page()
---
 fs/f2fs/data.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 94f7b084f601..f772fbc7f331 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2816,6 +2816,9 @@ int f2fs_write_single_data_page(struct folio *folio, int *submitted,
 
 	trace_f2fs_writepage(folio, DATA);
 
+	if (submitted)
+		*submitted = 0;
+
 	/* we should bypass data pages to proceed the kworker jobs */
 	if (unlikely(f2fs_cp_error(sbi))) {
 		mapping_set_error(folio->mapping, -EIO);
-- 
2.43.2



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v3] f2fs: fix using wrong 'submitted' value in f2fs_write_cache_pages

[Jaegeuk Kim via Linux-f2fs-devel]

On 01/06, zangyangyang wrote:
> From: zangyangyang1 <zangyangyang1@xiaomi.com>
> 
> When f2fs_write_single_data_page fails, f2fs_write_cache_pages
> will use the last 'submitted' value incorrectly, which will cause
> 'nwritten' and 'wbc->nr_to_write' calculation errors
> 
> Signed-off-by: zangyangyang1 <zangyangyang1@xiaomi.com>
> ---
> v3: No logical changes, just format patch
> v2: Initialize "submitted" in f2fs_write_single_data_page()
> ---
>  fs/f2fs/data.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 94f7b084f601..f772fbc7f331 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -2816,6 +2816,9 @@ int f2fs_write_single_data_page(struct folio *folio, int *submitted,
>  
>  	trace_f2fs_writepage(folio, DATA);
>  
> +	if (submitted)
> +		*submitted = 0;

I don't think this is correct, as callers should handle it.

> +
>  	/* we should bypass data pages to proceed the kworker jobs */
>  	if (unlikely(f2fs_cp_error(sbi))) {
>  		mapping_set_error(folio->mapping, -EIO);
> -- 
> 2.43.2


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v3] f2fs: fix using wrong 'submitted' value in f2fs_write_cache_pages

[臧阳阳]

Jaegeuk Kim <jaegeuk@kernel.org> 于2025年1月9日周四 02:29写道：

>
> On 01/06, zangyangyang wrote:
> > From: zangyangyang1 <zangyangyang1@xiaomi.com>
> >
> > When f2fs_write_single_data_page fails, f2fs_write_cache_pages
> > will use the last 'submitted' value incorrectly, which will cause
> > 'nwritten' and 'wbc->nr_to_write' calculation errors
> >
> > Signed-off-by: zangyangyang1 <zangyangyang1@xiaomi.com>
> > ---
> > v3: No logical changes, just format patch
> > v2: Initialize "submitted" in f2fs_write_single_data_page()
> > ---
> >  fs/f2fs/data.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 94f7b084f601..f772fbc7f331 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -2816,6 +2816,9 @@ int f2fs_write_single_data_page(struct folio *folio, int *submitted,
> >
> >       trace_f2fs_writepage(folio, DATA);
> >
> > +     if (submitted)
> > +             *submitted = 0;
>
> I don't think this is correct, as callers should handle it.

Hi, Chao, what do you think?

Thanks

>
> > +
> >       /* we should bypass data pages to proceed the kworker jobs */
> >       if (unlikely(f2fs_cp_error(sbi))) {
> >               mapping_set_error(folio->mapping, -EIO);
> > --
> > 2.43.2


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v3] f2fs: fix using wrong 'submitted' value in f2fs_write_cache_pages

[Chao Yu via Linux-f2fs-devel]

On 1/9/25 10:23, 臧阳阳 wrote:
> Jaegeuk Kim <jaegeuk@kernel.org> 于2025年1月9日周四 02:29写道：
> 
>>
>> On 01/06, zangyangyang wrote:
>>> From: zangyangyang1 <zangyangyang1@xiaomi.com>
>>>
>>> When f2fs_write_single_data_page fails, f2fs_write_cache_pages
>>> will use the last 'submitted' value incorrectly, which will cause
>>> 'nwritten' and 'wbc->nr_to_write' calculation errors
>>>
>>> Signed-off-by: zangyangyang1 <zangyangyang1@xiaomi.com>
>>> ---
>>> v3: No logical changes, just format patch
>>> v2: Initialize "submitted" in f2fs_write_single_data_page()
>>> ---
>>>   fs/f2fs/data.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>> index 94f7b084f601..f772fbc7f331 100644
>>> --- a/fs/f2fs/data.c
>>> +++ b/fs/f2fs/data.c
>>> @@ -2816,6 +2816,9 @@ int f2fs_write_single_data_page(struct folio *folio, int *submitted,
>>>
>>>        trace_f2fs_writepage(folio, DATA);
>>>
>>> +     if (submitted)
>>> +             *submitted = 0;
>>
>> I don't think this is correct, as callers should handle it.
> 
> Hi, Chao, what do you think?

Both are fine to me, previously I suggest to handle it in 
f2fs_write_single_data_page(), because I'm worried about we may miss to handle 
it in any possible caller in further.

Thanks,

> 
> Thanks
> 
>>
>>> +
>>>        /* we should bypass data pages to proceed the kworker jobs */
>>>        if (unlikely(f2fs_cp_error(sbi))) {
>>>                mapping_set_error(folio->mapping, -EIO);
>>> --
>>> 2.43.2



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v3] f2fs: fix using wrong 'submitted' value in f2fs_write_cache_pages

[YangYang Zang]

On Thu, Jan 9, 2025 at 6:19 PM Chao Yu <chao@kernel.org> wrote:
>
> On 1/9/25 10:23, 臧阳阳 wrote:
> > Jaegeuk Kim <jaegeuk@kernel.org> 于2025年1月9日周四 02:29写道：
> >
> >>
> >> On 01/06, zangyangyang wrote:
> >>> From: zangyangyang1 <zangyangyang1@xiaomi.com>
> >>>
> >>> When f2fs_write_single_data_page fails, f2fs_write_cache_pages
> >>> will use the last 'submitted' value incorrectly, which will cause
> >>> 'nwritten' and 'wbc->nr_to_write' calculation errors
> >>>
> >>> Signed-off-by: zangyangyang1 <zangyangyang1@xiaomi.com>
> >>> ---
> >>> v3: No logical changes, just format patch
> >>> v2: Initialize "submitted" in f2fs_write_single_data_page()
> >>> ---
> >>>   fs/f2fs/data.c | 3 +++
> >>>   1 file changed, 3 insertions(+)
> >>>
> >>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> >>> index 94f7b084f601..f772fbc7f331 100644
> >>> --- a/fs/f2fs/data.c
> >>> +++ b/fs/f2fs/data.c
> >>> @@ -2816,6 +2816,9 @@ int f2fs_write_single_data_page(struct folio *folio, int *submitted,
> >>>
> >>>        trace_f2fs_writepage(folio, DATA);
> >>>
> >>> +     if (submitted)
> >>> +             *submitted = 0;
> >>
> >> I don't think this is correct, as callers should handle it.
> >
> > Hi, Chao, what do you think?
>
> Both are fine to me, previously I suggest to handle it in
> f2fs_write_single_data_page(), because I'm worried about we may miss to handle
> it in any possible caller in further.

Thank you very much, Chao.

Hi, Kim, If you still think callers should handle it, I'll send a new patch.

Thanks,
>
> Thanks,
>
> >
> > Thanks
> >
> >>
> >>> +
> >>>        /* we should bypass data pages to proceed the kworker jobs */
> >>>        if (unlikely(f2fs_cp_error(sbi))) {
> >>>                mapping_set_error(folio->mapping, -EIO);
> >>> --
> >>> 2.43.2
>


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel