Hi Trond,
On 07/11/2017 05:53 PM, Trond Myklebust wrote:
> "perf lock" shows fairly heavy contention for the bit waitqueue locks
> when doing an I/O heavy workload.
> Use a bit to tell whether or not there has been contention for a lock
> so that we can optimise away the bit waitqueue options in those cases.
>
> Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> ---
> fs/nfs/pagelist.c | 17 ++++++++++++++++-
> include/linux/nfs_page.h | 2 ++
> 2 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfs/pagelist.c b/fs/nfs/pagelist.c
> index ce5f8d2875ae..046162b79b4b 100644
> --- a/fs/nfs/pagelist.c
> +++ b/fs/nfs/pagelist.c
> @@ -155,9 +155,12 @@ nfs_page_group_lock(struct nfs_page *req, bool nonblock)
> if (!test_and_set_bit(PG_HEADLOCK, &head->wb_flags))
> return 0;
>
> - if (!nonblock)
> + if (!nonblock) {
> + set_bit(PG_CONTENDED1, &head->wb_flags);
> + smp_mb__after_atomic();
> return wait_on_bit_lock(&head->wb_flags, PG_HEADLOCK,
> TASK_UNINTERRUPTIBLE);
> + }
>
> return -EAGAIN;
> }
> @@ -175,6 +178,10 @@ nfs_page_group_lock_wait(struct nfs_page *req)
>
> WARN_ON_ONCE(head != head->wb_head);
>
> + if (!test_bit(PG_HEADLOCK, &head->wb_flags))
> + return;
> + set_bit(PG_CONTENDED1, &head->wb_flags);
> + smp_mb__after_atomic();
> wait_on_bit(&head->wb_flags, PG_HEADLOCK,
> TASK_UNINTERRUPTIBLE);
> }
> @@ -193,6 +200,8 @@ nfs_page_group_unlock(struct nfs_page *req)
> smp_mb__before_atomic();
> clear_bit(PG_HEADLOCK, &head->wb_flags);
> smp_mb__after_atomic();
> + if (!test_bit(PG_CONTENDED1, &head->wb_flags))
> + return;
> wake_up_bit(&head->wb_flags, PG_HEADLOCK);
> }
>
> @@ -383,6 +392,8 @@ void nfs_unlock_request(struct nfs_page *req)
> smp_mb__before_atomic();
> clear_bit(PG_BUSY, &req->wb_flags);
> smp_mb__after_atomic();
> + if (!test_bit(PG_CONTENDED2, &req->wb_flags))
> + return;
> wake_up_bit(&req->wb_flags, PG_BUSY);
> }
>
> @@ -465,6 +476,10 @@ void nfs_release_request(struct nfs_page *req)
> int
> nfs_wait_on_request(struct nfs_page *req)
> {
> + if (!test_bit(PG_BUSY, &req->wb_flags))
> + return 0;
> + set_bit(PG_CONTENDED2, &req->wb_flags);
> + smp_mb__after_atomic();
> return wait_on_bit_io(&req->wb_flags, PG_BUSY,
> TASK_UNINTERRUPTIBLE);
> }
> diff --git a/include/linux/nfs_page.h b/include/linux/nfs_page.h
> index abbee2d15dce..d67b67ae6c8b 100644
> --- a/include/linux/nfs_page.h
> +++ b/include/linux/nfs_page.h
> @@ -33,6 +33,8 @@ enum {
> PG_UPTODATE, /* page group sync bit in read path */
> PG_WB_END, /* page group sync bit in write path */
> PG_REMOVE, /* page group sync bit in write path */
> + PG_CONTENDED1, /* Is someone waiting for a lock? */
> + PG_CONTENDED2, /* Is someone waiting for a lock? */
It looks like CONTENDED1 is for page groups, and CONTENDED2 is for requests? I wonder
if that could be a little more explicit either through this comment or through the names
of the variables themselves.
Thanks,
Anna
> };
>
> struct nfs_inode;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
