lkml.org 
[lkml]   [2020]   [Oct]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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.

\
 
 \ /
  Last update: 2020-10-28 23:09    [W:0.147 / U:0.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site