[PATCH v2 01/15] ubifs: Set page uptodate in the correct place

[Zhihao Cheng]

在 2024/1/25 1:52, Matthew Wilcox (Oracle) 写道:
> Page cache reads are lockless, so setting the freshly allocated page
> uptodate before we've overwritten it with the data it's supposed to have
> in it will allow a simultaneous reader to see old data.  Move the call
> to SetPageUptodate into ubifs_write_end(), which is after we copied the
> new data into the page.
> 
> Fixes: 1e51764a3c2a ("UBIFS: add new flash file system")
> Cc: stable at vger.kernel.org
> Signed-off-by: Matthew Wilcox (Oracle) <willy at infradead.org>
> ---
>   fs/ubifs/file.c | 13 ++++---------
>   1 file changed, 4 insertions(+), 9 deletions(-)

Reviewed-by: Zhihao Cheng <chengzhihao1 at huawei.com>

Just one nit below.

> 
> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index 5029eb3390a5..d0694b83dd02 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -261,9 +261,6 @@ static int write_begin_slow(struct address_space *mapping,
>   				return err;
>   			}
>   		}
> -
> -		SetPageUptodate(page);
> -		ClearPageError(page);
>   	}
>   
>   	if (PagePrivate(page))
> @@ -463,9 +460,6 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
>   				return err;
>   			}
>   		}
> -
> -		SetPageUptodate(page);
> -		ClearPageError(page);
>   	}
>   
>   	err = allocate_budget(c, page, ui, appending);
> @@ -475,10 +469,8 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
>   		 * If we skipped reading the page because we were going to
>   		 * write all of it, then it is not up to date.

It would be better to update above comments. For example, "If we skipped 
reading the page because we were going to write all of it, then 
PageChecked flag should be removed."

>   		 */
> -		if (skipped_read) {
> +		if (skipped_read)
>   			ClearPageChecked(page);
> -			ClearPageUptodate(page);
> -		}
>   		/*
>   		 * Budgeting failed which means it would have to force
>   		 * write-back but didn't, because we set the @fast flag in the
> @@ -569,6 +561,9 @@ static int ubifs_write_end(struct file *file, struct address_space *mapping,
>   		goto out;
>   	}
>   
> +	if (len == PAGE_SIZE)
> +		SetPageUptodate(page);
> +
>   	if (!PagePrivate(page)) {
>   		attach_page_private(page, (void *)1);
>   		atomic_long_inc(&c->dirty_pg_cnt);
>