| Message ID | 20240624042815.2242201-1-yangerkun@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [6.6~6.9] cifs: fix pagecache leak when do writepages | expand |
On Mon, Jun 24, 2024 at 12:28:15PM +0800, Yang Erkun wrote: > After commit f3dc1bdb6b0b("cifs: Fix writeback data corruption"), the > writepages for cifs will find all folio needed writepage with two phase. > The first folio will be found in cifs_writepages_begin, and the latter > various folios will be found in cifs_extend_writeback. > > All those will first get folio, and for normal case, once we set page > writeback and after do really write, we should put the reference, folio > found in cifs_extend_writeback do this with folio_batch_release. But the > folio found in cifs_writepages_begin never get the chance do it. And > every writepages call, we will leak a folio(found this problem while do > xfstests over cifs). > > Besides, the exist path seem never handle this folio correctly, fix it too > with this patch. > > The problem does not exist in mainline since writepages path for cifs > has changed to netfs. It's had to backport all related change, so try fix > this problem with this single patch. > > Fixes: f3dc1bdb6b0b ("cifs: Fix writeback data corruption") > Signed-off-by: Yang Erkun <yangerkun@huawei.com> > --- > fs/smb/client/file.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > <formletter> This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html for how to do this properly. </formletter>
在 2024/6/24 22:57, Greg KH 写道: > On Mon, Jun 24, 2024 at 12:28:15PM +0800, Yang Erkun wrote: >> After commit f3dc1bdb6b0b("cifs: Fix writeback data corruption"), the >> writepages for cifs will find all folio needed writepage with two phase. >> The first folio will be found in cifs_writepages_begin, and the latter >> various folios will be found in cifs_extend_writeback. >> >> All those will first get folio, and for normal case, once we set page >> writeback and after do really write, we should put the reference, folio >> found in cifs_extend_writeback do this with folio_batch_release. But the >> folio found in cifs_writepages_begin never get the chance do it. And >> every writepages call, we will leak a folio(found this problem while do >> xfstests over cifs). >> >> Besides, the exist path seem never handle this folio correctly, fix it too >> with this patch. >> >> The problem does not exist in mainline since writepages path for cifs >> has changed to netfs. It's had to backport all related change, so try fix >> this problem with this single patch. >> >> Fixes: f3dc1bdb6b0b ("cifs: Fix writeback data corruption") >> Signed-off-by: Yang Erkun <yangerkun@huawei.com> >> --- >> fs/smb/client/file.c | 16 +++++++++++++--- >> 1 file changed, 13 insertions(+), 3 deletions(-) >> > > <formletter> > > This is not the correct way to submit patches for inclusion in the > stable kernel tree. Please read: > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html > for how to do this properly. > > </formletter> Thanks for your reminder, will do it in v2!
diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c index 9be37d0fe724..0a48d80b3871 100644 --- a/fs/smb/client/file.c +++ b/fs/smb/client/file.c @@ -2860,17 +2860,21 @@ static ssize_t cifs_write_back_from_locked_folio(struct address_space *mapping, rc = cifs_get_writable_file(CIFS_I(inode), FIND_WR_ANY, &cfile); if (rc) { cifs_dbg(VFS, "No writable handle in writepages rc=%d\n", rc); + folio_unlock(folio); goto err_xid; } rc = server->ops->wait_mtu_credits(server, cifs_sb->ctx->wsize, &wsize, credits); - if (rc != 0) + if (rc != 0) { + folio_unlock(folio); goto err_close; + } wdata = cifs_writedata_alloc(cifs_writev_complete); if (!wdata) { rc = -ENOMEM; + folio_unlock(folio); goto err_uncredit; } @@ -3017,17 +3021,22 @@ static ssize_t cifs_writepages_begin(struct address_space *mapping, lock_again: if (wbc->sync_mode != WB_SYNC_NONE) { ret = folio_lock_killable(folio); - if (ret < 0) + if (ret < 0) { + folio_put(folio); return ret; + } } else { - if (!folio_trylock(folio)) + if (!folio_trylock(folio)) { + folio_put(folio); goto search_again; + } } if (folio->mapping != mapping || !folio_test_dirty(folio)) { start += folio_size(folio); folio_unlock(folio); + folio_put(folio); goto search_again; } @@ -3057,6 +3066,7 @@ static ssize_t cifs_writepages_begin(struct address_space *mapping, out: if (ret > 0) *_start = start + ret; + folio_put(folio); return ret; }
After commit f3dc1bdb6b0b("cifs: Fix writeback data corruption"), the writepages for cifs will find all folio needed writepage with two phase. The first folio will be found in cifs_writepages_begin, and the latter various folios will be found in cifs_extend_writeback. All those will first get folio, and for normal case, once we set page writeback and after do really write, we should put the reference, folio found in cifs_extend_writeback do this with folio_batch_release. But the folio found in cifs_writepages_begin never get the chance do it. And every writepages call, we will leak a folio(found this problem while do xfstests over cifs). Besides, the exist path seem never handle this folio correctly, fix it too with this patch. The problem does not exist in mainline since writepages path for cifs has changed to netfs. It's had to backport all related change, so try fix this problem with this single patch. Fixes: f3dc1bdb6b0b ("cifs: Fix writeback data corruption") Signed-off-by: Yang Erkun <yangerkun@huawei.com> --- fs/smb/client/file.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)