2017-01-03 18:22:58

by Nicholas Piggin

[permalink] [raw]
Subject: [PATCH 0/2] wake_up_page cleanups

I suggest getting acks for the nfs patch and merging these via
Andrew's tree.

Nicholas Piggin (2):
nfs: no PG_private waiters remain, remove waker
mm: un-export wake_up_page functions

fs/nfs/write.c | 2 --
include/linux/pagemap.h | 12 ++----------
mm/filemap.c | 10 ++++++++--
3 files changed, 10 insertions(+), 14 deletions(-)

--
2.11.0



2017-01-03 18:22:58

by Nicholas Piggin

[permalink] [raw]
Subject: [PATCH 1/2] nfs: no PG_private waiters remain, remove waker

Since commit 4f52b6bb ("NFS: Don't call COMMIT in ->releasepage()"),
no tasks wait on PagePrivate, so the wake introduced in commit 95905446
("NFS: avoid deadlocks with loop-back mounted NFS filesystems.") can
be removed.

Signed-off-by: Nicholas Piggin <[email protected]>
---
fs/nfs/write.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index b00d53d13d47..006068526542 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -728,8 +728,6 @@ static void nfs_inode_remove_request(struct nfs_page *req)
if (likely(head->wb_page && !PageSwapCache(head->wb_page))) {
set_page_private(head->wb_page, 0);
ClearPagePrivate(head->wb_page);
- smp_mb__after_atomic();
- wake_up_page(head->wb_page, PG_private);
clear_bit(PG_MAPPED, &head->wb_flags);
}
nfsi->nrequests--;
--
2.11.0


2017-01-03 18:23:01

by Nicholas Piggin

[permalink] [raw]
Subject: [PATCH 2/2] mm: un-export wake_up_page functions

These are no longer used outside mm/filemap.c, so un-export them and
make them static where possible. These were exported specifically for
NFS use in commit a4796e37c12e ("MM: export page_wakeup functions").

Signed-off-by: Nicholas Piggin <[email protected]>
---
include/linux/pagemap.h | 12 ++----------
mm/filemap.c | 10 ++++++++--
2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 324c8dbad1e1..b572f5530392 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -482,19 +482,11 @@ static inline int lock_page_or_retry(struct page *page, struct mm_struct *mm,
}

/*
- * This is exported only for wait_on_page_locked/wait_on_page_writeback,
- * and for filesystems which need to wait on PG_private.
+ * This is exported only for wait_on_page_locked/wait_on_page_writeback, etc.,
+ * and should not be used directly.
*/
extern void wait_on_page_bit(struct page *page, int bit_nr);
extern int wait_on_page_bit_killable(struct page *page, int bit_nr);
-extern void wake_up_page_bit(struct page *page, int bit_nr);
-
-static inline void wake_up_page(struct page *page, int bit)
-{
- if (!PageWaiters(page))
- return;
- wake_up_page_bit(page, bit);
-}

/*
* Wait for a page to be unlocked.
diff --git a/mm/filemap.c b/mm/filemap.c
index d0e4d1002059..52115b967688 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -788,7 +788,7 @@ static int wake_page_function(wait_queue_t *wait, unsigned mode, int sync, void
return autoremove_wake_function(wait, mode, sync, key);
}

-void wake_up_page_bit(struct page *page, int bit_nr)
+static void wake_up_page_bit(struct page *page, int bit_nr)
{
wait_queue_head_t *q = page_waitqueue(page);
struct wait_page_key key;
@@ -821,7 +821,13 @@ void wake_up_page_bit(struct page *page, int bit_nr)
}
spin_unlock_irqrestore(&q->lock, flags);
}
-EXPORT_SYMBOL(wake_up_page_bit);
+
+static void wake_up_page(struct page *page, int bit)
+{
+ if (!PageWaiters(page))
+ return;
+ wake_up_page_bit(page, bit);
+}

static inline int wait_on_page_bit_common(wait_queue_head_t *q,
struct page *page, int bit_nr, int state, bool lock)
--
2.11.0


2017-01-04 13:43:16

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH 1/2] nfs: no PG_private waiters remain, remove waker

Hi Nick,

> On Jan 3, 2017, at 13:22, Nicholas Piggin <[email protected]> wrote:
>=20
> Since commit 4f52b6bb ("NFS: Don't call COMMIT in ->releasepage()"),
> no tasks wait on PagePrivate, so the wake introduced in commit 95905446
> ("NFS: avoid deadlocks with loop-back mounted NFS filesystems.") can
> be removed.
>=20
> Signed-off-by: Nicholas Piggin <[email protected]>
> ---
> fs/nfs/write.c | 2 --
> 1 file changed, 2 deletions(-)
>=20
> diff --git a/fs/nfs/write.c b/fs/nfs/write.c
> index b00d53d13d47..006068526542 100644
> --- a/fs/nfs/write.c
> +++ b/fs/nfs/write.c
> @@ -728,8 +728,6 @@ static void nfs_inode_remove_request(struct nfs_page =
*req)
> =09=09if (likely(head->wb_page && !PageSwapCache(head->wb_page))) {
> =09=09=09set_page_private(head->wb_page, 0);
> =09=09=09ClearPagePrivate(head->wb_page);
> -=09=09=09smp_mb__after_atomic();
> -=09=09=09wake_up_page(head->wb_page, PG_private);
> =09=09=09clear_bit(PG_MAPPED, &head->wb_flags);
> =09=09}
> =09=09nfsi->nrequests--;
> --=20
> 2.11.0
>=20

That looks fine to me. Do you want to push it through the linux-mm path or =
do you want me to take it?

Cheers
Trond

Acked-by: Trond Myklebust <[email protected]>


2017-01-05 05:08:22

by Nicholas Piggin

[permalink] [raw]
Subject: Re: [PATCH 1/2] nfs: no PG_private waiters remain, remove waker

On Wed, 4 Jan 2017 13:43:10 +0000
Trond Myklebust <[email protected]> wrote:

> Hi Nick,
>
> > On Jan 3, 2017, at 13:22, Nicholas Piggin <[email protected]> wrote:
> >
> > Since commit 4f52b6bb ("NFS: Don't call COMMIT in ->releasepage()"),
> > no tasks wait on PagePrivate, so the wake introduced in commit 95905446
> > ("NFS: avoid deadlocks with loop-back mounted NFS filesystems.") can
> > be removed.
> >
> > Signed-off-by: Nicholas Piggin <[email protected]>
> > ---
> > fs/nfs/write.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/fs/nfs/write.c b/fs/nfs/write.c
> > index b00d53d13d47..006068526542 100644
> > --- a/fs/nfs/write.c
> > +++ b/fs/nfs/write.c
> > @@ -728,8 +728,6 @@ static void nfs_inode_remove_request(struct nfs_page *req)
> > if (likely(head->wb_page && !PageSwapCache(head->wb_page))) {
> > set_page_private(head->wb_page, 0);
> > ClearPagePrivate(head->wb_page);
> > - smp_mb__after_atomic();
> > - wake_up_page(head->wb_page, PG_private);
> > clear_bit(PG_MAPPED, &head->wb_flags);
> > }
> > nfsi->nrequests--;
> > --
> > 2.11.0
> >
>
> That looks fine to me. Do you want to push it through the linux-mm path or do you want me to take it?

Hi Trond,

Thanks. I don't see a problem with both patches going through your tree.
I think the patches to add this stuff went through your tree as well.
The removal of the export is really the only thing that makes patch 2
non-trivial, but considering it was added for NFS, I think it's safe to
remove.

Thanks,
Nick