| Message ID | 20240124175302.1750912-2-willy@infradead.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | ubifs folio conversion | expand |
在 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@vger.kernel.org > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > fs/ubifs/file.c | 13 ++++--------- > 1 file changed, 4 insertions(+), 9 deletions(-) Reviewed-by: Zhihao Cheng <chengzhihao1@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); >
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. */ - 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);
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@vger.kernel.org Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/ubifs/file.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)