2014-05-12 01:23:32

by NeilBrown

[permalink] [raw]
Subject: [PATCH 0/2] NFSD: restrict PF_LESS_THROTTLE to loop-back requests.

As PF_LESS_THROTTLE now has a slightly broader meaning it makes sense
to restrict it to only the times it is actually needed - when write
requests come from the local host.

Thanks,
NeilBrown

---

NeilBrown (2):
SUNRPC: track whether a request is coming from a loop-back interface.
nfsd: Only set PF_LESS_THROTTLE when really needed.


fs/nfsd/nfssvc.c | 6 ------
fs/nfsd/vfs.c | 12 ++++++++++++
include/linux/sunrpc/svc.h | 1 +
include/linux/sunrpc/svc_xprt.h | 1 +
net/sunrpc/sunrpc.h | 13 +++++++++++++
net/sunrpc/svcsock.c | 5 +++++
6 files changed, 32 insertions(+), 6 deletions(-)

--
Signature



2014-05-22 19:59:37

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 0/2] NFSD: restrict PF_LESS_THROTTLE to loop-back requests.

On Mon, May 12, 2014 at 11:22:47AM +1000, NeilBrown wrote:
> As PF_LESS_THROTTLE now has a slightly broader meaning it makes sense
> to restrict it to only the times it is actually needed - when write
> requests come from the local host.

Both look good to me, thanks. Applying for 3.16.

--b.

>
> Thanks,
> NeilBrown
>
> ---
>
> NeilBrown (2):
> SUNRPC: track whether a request is coming from a loop-back interface.
> nfsd: Only set PF_LESS_THROTTLE when really needed.
>
>
> fs/nfsd/nfssvc.c | 6 ------
> fs/nfsd/vfs.c | 12 ++++++++++++
> include/linux/sunrpc/svc.h | 1 +
> include/linux/sunrpc/svc_xprt.h | 1 +
> net/sunrpc/sunrpc.h | 13 +++++++++++++
> net/sunrpc/svcsock.c | 5 +++++
> 6 files changed, 32 insertions(+), 6 deletions(-)
>
> --
> Signature
>

2014-05-12 01:23:38

by NeilBrown

[permalink] [raw]
Subject: [PATCH 1/2] SUNRPC: track whether a request is coming from a loop-back interface.

If an incoming NFS request is coming from the local host, then
nfsd will need to perform some special handling. So detect that
possibility and make the source visible in rq_local.

Signed-off-by: NeilBrown <[email protected]>
---
include/linux/sunrpc/svc.h | 1 +
include/linux/sunrpc/svc_xprt.h | 1 +
net/sunrpc/sunrpc.h | 13 +++++++++++++
net/sunrpc/svcsock.c | 5 +++++
4 files changed, 20 insertions(+)

diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 04e763221246..a0dbbd1e00e9 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -254,6 +254,7 @@ struct svc_rqst {
u32 rq_prot; /* IP protocol */
unsigned short
rq_secure : 1; /* secure port */
+ unsigned short rq_local : 1; /* local request */

void * rq_argp; /* decoded arguments */
void * rq_resp; /* xdr'd results */
diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index b05963f09ebf..b99bdfb0fcf9 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -63,6 +63,7 @@ struct svc_xprt {
#define XPT_DETACHED 10 /* detached from tempsocks list */
#define XPT_LISTENER 11 /* listening endpoint */
#define XPT_CACHE_AUTH 12 /* cache auth info */
+#define XPT_LOCAL 13 /* connection from loopback interface */

struct svc_serv *xpt_server; /* service for transport */
atomic_t xpt_reserved; /* space on outq that is rsvd */
diff --git a/net/sunrpc/sunrpc.h b/net/sunrpc/sunrpc.h
index 14c9f6d1c5ff..f2b7cb540e61 100644
--- a/net/sunrpc/sunrpc.h
+++ b/net/sunrpc/sunrpc.h
@@ -43,6 +43,19 @@ static inline int rpc_reply_expected(struct rpc_task *task)
(task->tk_msg.rpc_proc->p_decode != NULL);
}

+static inline int sock_is_loopback(struct sock *sk)
+{
+ struct dst_entry *dst;
+ int loopback = 0;
+ rcu_read_lock();
+ dst = rcu_dereference(sk->sk_dst_cache);
+ if (dst && dst->dev &&
+ (dst->dev->features & NETIF_F_LOOPBACK))
+ loopback = 1;
+ rcu_read_unlock();
+ return loopback;
+}
+
int svc_send_common(struct socket *sock, struct xdr_buf *xdr,
struct page *headpage, unsigned long headoffset,
struct page *tailpage, unsigned long tailoffset);
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 43bcb4699d69..95684986c372 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -867,6 +867,10 @@ static struct svc_xprt *svc_tcp_accept(struct svc_xprt *xprt)
}
svc_xprt_set_local(&newsvsk->sk_xprt, sin, slen);

+ if (sock_is_loopback(newsock->sk))
+ set_bit(XPT_LOCAL, &newsvsk->sk_xprt.xpt_flags);
+ else
+ clear_bit(XPT_LOCAL, &newsvsk->sk_xprt.xpt_flags);
if (serv->sv_stats)
serv->sv_stats->nettcpconn++;

@@ -1112,6 +1116,7 @@ static int svc_tcp_recvfrom(struct svc_rqst *rqstp)

rqstp->rq_xprt_ctxt = NULL;
rqstp->rq_prot = IPPROTO_TCP;
+ rqstp->rq_local = !!test_bit(XPT_LOCAL, &svsk->sk_xprt.xpt_flags);

p = (__be32 *)rqstp->rq_arg.head[0].iov_base;
calldir = p[1];



2014-05-12 01:23:44

by NeilBrown

[permalink] [raw]
Subject: [PATCH 2/2] nfsd: Only set PF_LESS_THROTTLE when really needed.

PF_LESS_THROTTLE has a very specific use case: to avoid deadlocks
and live-locks while writing to the page cache in a loop-back
NFS mount situation.

It therefore makes sense to *only* set PF_LESS_THROTTLE in this
situation.
We now know when a request came from the local-host so it could be a
loop-back mount. We already know when we are handling write requests,
and when we are doing anything else.

So combine those two to allow nfsd to still be throttled (like any
other process) in every situation except when it is known to be
problematic.

Signed-off-by: NeilBrown <[email protected]>
---
fs/nfsd/nfssvc.c | 6 ------
fs/nfsd/vfs.c | 12 ++++++++++++
2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index 9a4a5f9e7468..1879e43f2868 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -591,12 +591,6 @@ nfsd(void *vrqstp)
nfsdstats.th_cnt++;
mutex_unlock(&nfsd_mutex);

- /*
- * We want less throttling in balance_dirty_pages() so that nfs to
- * localhost doesn't cause nfsd to lock up due to all the client's
- * dirty pages.
- */
- current->flags |= PF_LESS_THROTTLE;
set_freezable();

/*
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 16f0673a423c..b05d21e93d9a 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -922,6 +922,16 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct file *file,
int stable = *stablep;
int use_wgather;
loff_t pos = offset;
+ unsigned int pflags = current->flags;
+
+ if (rqstp->rq_local)
+ /*
+ * We want less throttling in balance_dirty_pages()
+ * and shrink_inactive_list() so that nfs to
+ * localhost doesn't cause nfsd to lock up due to all
+ * the client's dirty pages or its congested queue.
+ */
+ current->flags |= PF_LESS_THROTTLE;

dentry = file->f_path.dentry;
inode = dentry->d_inode;
@@ -959,6 +969,8 @@ out_nfserr:
err = 0;
else
err = nfserrno(host_err);
+ if (rqstp->rq_local)
+ tsk_restore_flags(current, pflags, PF_LESS_THROTTLE);
return err;
}