| Message ID | 20240830032747.1082484-1-zhaoyang.huang@unisoc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] fs: nfs: fix missing refcnt by replacing folio_set_private by folio_attach_private | expand |
Hi Anna and Trond, Please drop '03e02b94171b1985dd0aa184296fe94425b855a3' fs: nfs: fix missing refcnt by replacing folio_set_private by folio_attach_private since it is wrong as Trond has explained. Thank you! On Tue, Sep 3, 2024 at 2:00 AM Trond Myklebust <trondmy@hammerspace.com> wrote: > > On Mon, 2024-09-02 at 10:42 +0800, Zhaoyang Huang wrote: > > Hi Trond, > > I am not sure if 'trondmy@kernel.org' is still available or if you > > are > > too busy to respond. Try these two addresses. > > > > I believe that I already answered this on the list. The refcount that > this change adds is redundant because the struct nfs_page already holds > its own refcount to the same folio. > > Thanks! > > > ---------- Forwarded message --------- > > From: zhaoyang.huang <zhaoyang.huang@unisoc.com> > > Date: Fri, Aug 30, 2024 at 11:28 AM > > Subject: [PATCH 1/1] fs: nfs: fix missing refcnt by replacing > > folio_set_private by folio_attach_private > > To: Trond Myklebust <trondmy@kernel.org>, Anna Schumaker > > <anna@kernel.org>, Christoph Hellwig <hch@infradead.org>, > > <linux-nfs@vger.kernel.org>, <linux-kernel@vger.kernel.org>, Zhaoyang > > Huang <huangzhaoyang@gmail.com>, <steve.kang@unisoc.com> > > > > > > From: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > > > > This patch is inspired by a code review of fs codes which aims at > > folio's extra refcnt that could introduce unwanted behavious when > > judging refcnt, such as[1].That is, the folio passed to > > mapping_evict_folio carries the refcnts from find_lock_entries, > > page_cache, corresponding to PTEs and folio's private if has. > > However, > > current code doesn't take the refcnt for folio's private which could > > have mapping_evict_folio miss the one to only PTE and lead to > > call filemap_release_folio wrongly. > > > > [1] > > long mapping_evict_folio(struct address_space *mapping, struct folio > > *folio) > > { > > ... > > //current code will misjudge here if there is one pte on the folio > > which > > is be deemed as the one as folio's private > > if (folio_ref_count(folio) > > > folio_nr_pages(folio) + > > folio_has_private(folio) + 1) > > return 0; > > if (!filemap_release_folio(folio, 0)) > > return 0; > > > > return remove_mapping(mapping, folio); > > } > > > > Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > > --- > > fs/nfs/write.c | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/fs/nfs/write.c b/fs/nfs/write.c > > index d074d0ceb4f0..80c6ded5f74c 100644 > > --- a/fs/nfs/write.c > > +++ b/fs/nfs/write.c > > @@ -772,8 +772,7 @@ static void nfs_inode_add_request(struct nfs_page > > *req) > > nfs_lock_request(req); > > spin_lock(&mapping->i_private_lock); > > set_bit(PG_MAPPED, &req->wb_flags); > > - folio_set_private(folio); > > - folio->private = req; > > + folio_attach_private(folio, req); > > spin_unlock(&mapping->i_private_lock); > > atomic_long_inc(&nfsi->nrequests); > > /* this a head request for a page group - mark it as having > > an > > @@ -797,8 +796,7 @@ static void nfs_inode_remove_request(struct > > nfs_page *req) > > > > spin_lock(&mapping->i_private_lock); > > if (likely(folio)) { > > - folio->private = NULL; > > - folio_clear_private(folio); > > + folio_detach_private(folio); > > clear_bit(PG_MAPPED, &req->wb_head- > > >wb_flags); > > } > > spin_unlock(&mapping->i_private_lock); > > -- > > 2.25.1 > > -- > Trond Myklebust > Linux NFS client maintainer, Hammerspace > trond.myklebust@hammerspace.com > >
diff --git a/fs/nfs/write.c b/fs/nfs/write.c index d074d0ceb4f0..80c6ded5f74c 100644 --- a/fs/nfs/write.c +++ b/fs/nfs/write.c @@ -772,8 +772,7 @@ static void nfs_inode_add_request(struct nfs_page *req) nfs_lock_request(req); spin_lock(&mapping->i_private_lock); set_bit(PG_MAPPED, &req->wb_flags); - folio_set_private(folio); - folio->private = req; + folio_attach_private(folio, req); spin_unlock(&mapping->i_private_lock); atomic_long_inc(&nfsi->nrequests); /* this a head request for a page group - mark it as having an @@ -797,8 +796,7 @@ static void nfs_inode_remove_request(struct nfs_page *req) spin_lock(&mapping->i_private_lock); if (likely(folio)) { - folio->private = NULL; - folio_clear_private(folio); + folio_detach_private(folio); clear_bit(PG_MAPPED, &req->wb_head->wb_flags); } spin_unlock(&mapping->i_private_lock);