Messages in this thread Patch in this message | | | Date | Wed, 28 Oct 2020 14:20:41 +0000 | From | Matthew Wilcox <> | Subject | Re: [PATCH 05/11] afs: Fix to take ref on page when PG_private is set |
| |
On Wed, Oct 28, 2020 at 02:10:24PM +0000, David Howells wrote: > +++ b/fs/afs/dir.c > @@ -283,6 +283,7 @@ static struct afs_read *afs_read_dir(struct afs_vnode *dvnode, struct key *key) > > set_page_private(req->pages[i], 1); > SetPagePrivate(req->pages[i]); > + get_page(req->pages[i]);
Alternative spelling:
- set_page_private(req->pages[i], 1); - SetPagePrivate(req->pages[i]); + attach_page_private(req->pages[i], (void *)1);
AFS is an anomaly; most filesystems actually stick a pointer in page->private.
> +++ b/fs/afs/write.c > @@ -151,7 +151,8 @@ int afs_write_begin(struct file *file, struct address_space *mapping, > priv |= f; > trace_afs_page_dirty(vnode, tracepoint_string("begin"), > page->index, priv); > - SetPagePrivate(page); > + if (!TestSetPagePrivate(page)) > + get_page(page); > set_page_private(page, priv); > _leave(" = 0"); > return 0;
There's an efficiency question here that I can't answer ... how often do you call afs_write_begin() on a page which already has PagePrivate set? It's fewer atomic ops to do:
if (PagePrivate(page)) set_page_private(page, priv); else attach_page_private(page, (void *)priv);
I have no objection to adding TestSetPagePrivate per se; I just don't know if it's really what you want or not.
| |