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.
