2010-09-28 15:15:04

by Pavel Emelyanov

[permalink] [raw]
Subject: [PATCH v2 0/7] sunrpc: Preparations to create sockets in namespaces

Fixed comments from Chuck and Trond.
Added one patch, that factors out rpc_xprt allocation as well.

Patches, that were #7 and #8 await for Dave's approval :)

Thanks,
Pavel


2010-09-28 15:41:02

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 2/7] sunrpc: Factor out rpc_xprt freeing


On Sep 28, 2010, at 11:15 AM, Pavel Emelyanov wrote:

> Signed-off-by: Pavel Emelyanov <[email protected]>
> ---
> include/linux/sunrpc/xprt.h | 1 +
> net/sunrpc/xprtrdma/transport.c | 7 ++-----
> net/sunrpc/xprtsock.c | 19 +++++++++++--------
> 3 files changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
> index 00f6e3f..af4b560 100644
> --- a/include/linux/sunrpc/xprt.h
> +++ b/include/linux/sunrpc/xprt.h
> @@ -281,6 +281,7 @@ void xprt_release(struct rpc_task *task);
> struct rpc_xprt * xprt_get(struct rpc_xprt *xprt);
> void xprt_put(struct rpc_xprt *xprt);
> struct rpc_xprt * xprt_alloc(int size, int max_req);
> +void xprt_free(struct rpc_xprt *);
>
> static inline __be32 *xprt_skip_transport_header(struct rpc_xprt *xprt, __be32 *p)
> {
> diff --git a/net/sunrpc/xprtrdma/transport.c b/net/sunrpc/xprtrdma/transport.c
> index 9d77bf2..0f7a1b9 100644
> --- a/net/sunrpc/xprtrdma/transport.c
> +++ b/net/sunrpc/xprtrdma/transport.c
> @@ -251,9 +251,7 @@ xprt_rdma_destroy(struct rpc_xprt *xprt)
>
> xprt_rdma_free_addresses(xprt);
>
> - kfree(xprt->slot);
> - xprt->slot = NULL;
> - kfree(xprt);
> + xprt_free(xprt);
>
> dprintk("RPC: %s: returning\n", __func__);
>
> @@ -401,8 +399,7 @@ out3:
> out2:
> rpcrdma_ia_close(&new_xprt->rx_ia);
> out1:
> - kfree(xprt->slot);
> - kfree(xprt);
> + xprt_free(xprt);
> return ERR_PTR(rc);
> }
>
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 9f2dd3b..77cb595 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -781,6 +781,13 @@ out:
> }
> EXPORT_SYMBOL_GPL(xprt_alloc);
>
> +void xprt_free(struct rpc_xprt *xprt)
> +{
> + kfree(xprt->slot);
> + kfree(xprt);
> +}
> +EXPORT_SYMBOL_GPL(xprt_free);

Likewise: I think xprt_free, being a generic API, belongs in net/sunrpc/xprt.c not in xprtsock.c

If the xprt and slot table were allocated with a single kcalloc request, for example, then you probably wouldn't need xprt_free at all. Not saying that's a requirement, but simply making an observation.

> +
> /**
> * xs_destroy - prepare to shutdown a transport
> * @xprt: doomed transport
> @@ -796,8 +803,7 @@ static void xs_destroy(struct rpc_xprt *xprt)
>
> xs_close(xprt);
> xs_free_peer_addresses(xprt);
> - kfree(xprt->slot);
> - kfree(xprt);
> + xprt_free(xprt);
> module_put(THIS_MODULE);
> }
>
> @@ -2384,8 +2390,7 @@ static struct rpc_xprt *xs_setup_udp(struct xprt_create *args)
> return xprt;
> ret = ERR_PTR(-EINVAL);
> out_err:
> - kfree(xprt->slot);
> - kfree(xprt);
> + xprt_free(xprt);
> return ret;
> }
>
> @@ -2460,8 +2465,7 @@ static struct rpc_xprt *xs_setup_tcp(struct xprt_create *args)
> return xprt;
> ret = ERR_PTR(-EINVAL);
> out_err:
> - kfree(xprt->slot);
> - kfree(xprt);
> + xprt_free(xprt);
> return ret;
> }
>
> @@ -2541,8 +2545,7 @@ static struct rpc_xprt *xs_setup_bc_tcp(struct xprt_create *args)
> return xprt;
> ret = ERR_PTR(-EINVAL);
> out_err:
> - kfree(xprt->slot);
> - kfree(xprt);
> + xprt_free(xprt);
> return ret;
> }
>
> --
> 1.5.5.6
>

--
chuck[dot]lever[at]oracle[dot]com





2010-09-28 15:17:53

by Pavel Emelyanov

[permalink] [raw]
Subject: [PATCH 6/7] sunrpc: Add net to xprt_create

Signed-off-by: Pavel Emelyanov <[email protected]>
---
include/linux/sunrpc/xprt.h | 1 +
net/sunrpc/clnt.c | 1 +
2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
index af4b560..c4f9315 100644
--- a/include/linux/sunrpc/xprt.h
+++ b/include/linux/sunrpc/xprt.h
@@ -249,6 +249,7 @@ static inline int bc_prealloc(struct rpc_rqst *req)

struct xprt_create {
int ident; /* XPRT_TRANSPORT identifier */
+ struct net * net;
struct sockaddr * srcaddr; /* optional local address */
struct sockaddr * dstaddr; /* remote peer address */
size_t addrlen;
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index fa55490..f4bbd83 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -284,6 +284,7 @@ struct rpc_clnt *rpc_create(struct rpc_create_args *args)
struct rpc_xprt *xprt;
struct rpc_clnt *clnt;
struct xprt_create xprtargs = {
+ .net = args->net,
.ident = args->protocol,
.srcaddr = args->saddress,
.dstaddr = args->address,
--
1.5.5.6


2010-09-28 15:15:58

by Pavel Emelyanov

[permalink] [raw]
Subject: [PATCH 2/7] sunrpc: Factor out rpc_xprt freeing

Signed-off-by: Pavel Emelyanov <[email protected]>
---
include/linux/sunrpc/xprt.h | 1 +
net/sunrpc/xprtrdma/transport.c | 7 ++-----
net/sunrpc/xprtsock.c | 19 +++++++++++--------
3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
index 00f6e3f..af4b560 100644
--- a/include/linux/sunrpc/xprt.h
+++ b/include/linux/sunrpc/xprt.h
@@ -281,6 +281,7 @@ void xprt_release(struct rpc_task *task);
struct rpc_xprt * xprt_get(struct rpc_xprt *xprt);
void xprt_put(struct rpc_xprt *xprt);
struct rpc_xprt * xprt_alloc(int size, int max_req);
+void xprt_free(struct rpc_xprt *);

static inline __be32 *xprt_skip_transport_header(struct rpc_xprt *xprt, __be32 *p)
{
diff --git a/net/sunrpc/xprtrdma/transport.c b/net/sunrpc/xprtrdma/transport.c
index 9d77bf2..0f7a1b9 100644
--- a/net/sunrpc/xprtrdma/transport.c
+++ b/net/sunrpc/xprtrdma/transport.c
@@ -251,9 +251,7 @@ xprt_rdma_destroy(struct rpc_xprt *xprt)

xprt_rdma_free_addresses(xprt);

- kfree(xprt->slot);
- xprt->slot = NULL;
- kfree(xprt);
+ xprt_free(xprt);

dprintk("RPC: %s: returning\n", __func__);

@@ -401,8 +399,7 @@ out3:
out2:
rpcrdma_ia_close(&new_xprt->rx_ia);
out1:
- kfree(xprt->slot);
- kfree(xprt);
+ xprt_free(xprt);
return ERR_PTR(rc);
}

diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 9f2dd3b..77cb595 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -781,6 +781,13 @@ out:
}
EXPORT_SYMBOL_GPL(xprt_alloc);

+void xprt_free(struct rpc_xprt *xprt)
+{
+ kfree(xprt->slot);
+ kfree(xprt);
+}
+EXPORT_SYMBOL_GPL(xprt_free);
+
/**
* xs_destroy - prepare to shutdown a transport
* @xprt: doomed transport
@@ -796,8 +803,7 @@ static void xs_destroy(struct rpc_xprt *xprt)

xs_close(xprt);
xs_free_peer_addresses(xprt);
- kfree(xprt->slot);
- kfree(xprt);
+ xprt_free(xprt);
module_put(THIS_MODULE);
}

@@ -2384,8 +2390,7 @@ static struct rpc_xprt *xs_setup_udp(struct xprt_create *args)
return xprt;
ret = ERR_PTR(-EINVAL);
out_err:
- kfree(xprt->slot);
- kfree(xprt);
+ xprt_free(xprt);
return ret;
}

@@ -2460,8 +2465,7 @@ static struct rpc_xprt *xs_setup_tcp(struct xprt_create *args)
return xprt;
ret = ERR_PTR(-EINVAL);
out_err:
- kfree(xprt->slot);
- kfree(xprt);
+ xprt_free(xprt);
return ret;
}

@@ -2541,8 +2545,7 @@ static struct rpc_xprt *xs_setup_bc_tcp(struct xprt_create *args)
return xprt;
ret = ERR_PTR(-EINVAL);
out_err:
- kfree(xprt->slot);
- kfree(xprt);
+ xprt_free(xprt);
return ret;
}

--
1.5.5.6


2010-09-28 15:15:34

by Pavel Emelyanov

[permalink] [raw]
Subject: [PATCH 1/7] sunrpc: Factor out rpc_xprt allocation

Signed-off-by: Pavel Emelyanov <[email protected]>

---
include/linux/sunrpc/xprt.h | 1 +
net/sunrpc/xprtrdma/transport.c | 13 ++-----------
net/sunrpc/xprtsock.c | 37 +++++++++++++++++++++++++------------
3 files changed, 28 insertions(+), 23 deletions(-)

diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
index ff5a77b..00f6e3f 100644
--- a/include/linux/sunrpc/xprt.h
+++ b/include/linux/sunrpc/xprt.h
@@ -280,6 +280,7 @@ void xprt_release_xprt_cong(struct rpc_xprt *xprt, struct rpc_task *task);
void xprt_release(struct rpc_task *task);
struct rpc_xprt * xprt_get(struct rpc_xprt *xprt);
void xprt_put(struct rpc_xprt *xprt);
+struct rpc_xprt * xprt_alloc(int size, int max_req);

static inline __be32 *xprt_skip_transport_header(struct rpc_xprt *xprt, __be32 *p)
{
diff --git a/net/sunrpc/xprtrdma/transport.c b/net/sunrpc/xprtrdma/transport.c
index a85e866..9d77bf2 100644
--- a/net/sunrpc/xprtrdma/transport.c
+++ b/net/sunrpc/xprtrdma/transport.c
@@ -285,23 +285,14 @@ xprt_setup_rdma(struct xprt_create *args)
return ERR_PTR(-EBADF);
}

- xprt = kzalloc(sizeof(struct rpcrdma_xprt), GFP_KERNEL);
+ xprt = xprt_alloc(sizeof(struct rpcrdma_xprt),
+ xprt_rdma_slot_table_entries);
if (xprt == NULL) {
dprintk("RPC: %s: couldn't allocate rpcrdma_xprt\n",
__func__);
return ERR_PTR(-ENOMEM);
}

- xprt->max_reqs = xprt_rdma_slot_table_entries;
- xprt->slot = kcalloc(xprt->max_reqs,
- sizeof(struct rpc_rqst), GFP_KERNEL);
- if (xprt->slot == NULL) {
- dprintk("RPC: %s: couldn't allocate %d slots\n",
- __func__, xprt->max_reqs);
- kfree(xprt);
- return ERR_PTR(-ENOMEM);
- }
-
/* 60 second timeout, no retries */
xprt->timeout = &xprt_rdma_default_timeout;
xprt->bind_timeout = (60U * HZ);
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index b6309db..9f2dd3b 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -759,6 +759,28 @@ static void xs_tcp_close(struct rpc_xprt *xprt)
xs_tcp_shutdown(xprt);
}

+struct rpc_xprt *xprt_alloc(int size, int max_req)
+{
+ struct rpc_xprt *xprt;
+
+ xprt = kzalloc(size, GFP_KERNEL);
+ if (xprt == NULL)
+ goto out;
+
+ xprt->max_reqs = max_req;
+ xprt->slot = kcalloc(max_req, sizeof(struct rpc_rqst), GFP_KERNEL);
+ if (xprt->slot == NULL)
+ goto out_free;
+
+ return xprt;
+
+out_free:
+ kfree(xprt);
+out:
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(xprt_alloc);
+
/**
* xs_destroy - prepare to shutdown a transport
* @xprt: doomed transport
@@ -2273,23 +2295,14 @@ static struct rpc_xprt *xs_setup_xprt(struct xprt_create *args,
return ERR_PTR(-EBADF);
}

- new = kzalloc(sizeof(*new), GFP_KERNEL);
- if (new == NULL) {
+ xprt = xprt_alloc(sizeof(*new), slot_table_size);
+ if (xprt == NULL) {
dprintk("RPC: xs_setup_xprt: couldn't allocate "
"rpc_xprt\n");
return ERR_PTR(-ENOMEM);
}
- xprt = &new->xprt;
-
- xprt->max_reqs = slot_table_size;
- xprt->slot = kcalloc(xprt->max_reqs, sizeof(struct rpc_rqst), GFP_KERNEL);
- if (xprt->slot == NULL) {
- kfree(xprt);
- dprintk("RPC: xs_setup_xprt: couldn't allocate slot "
- "table\n");
- return ERR_PTR(-ENOMEM);
- }

+ new = container_of(xprt, struct sock_xprt, xprt);
memcpy(&xprt->addr, args->dstaddr, args->addrlen);
xprt->addrlen = args->addrlen;
if (args->srcaddr)
--
1.5.5.6


2010-09-28 15:17:23

by Pavel Emelyanov

[permalink] [raw]
Subject: [PATCH 5/7] sunrpc: Add net to rpc_create_args

Signed-off-by: Pavel Emelyanov <[email protected]>
---
fs/lockd/host.c | 1 +
fs/lockd/mon.c | 1 +
fs/nfs/client.c | 1 +
fs/nfs/mount_clnt.c | 2 ++
fs/nfsd/nfs4callback.c | 1 +
include/linux/sunrpc/clnt.h | 1 +
net/sunrpc/rpcb_clnt.c | 2 ++
7 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/fs/lockd/host.c b/fs/lockd/host.c
index bb464d1..25e21e4 100644
--- a/fs/lockd/host.c
+++ b/fs/lockd/host.c
@@ -353,6 +353,7 @@ nlm_bind_host(struct nlm_host *host)
.to_retries = 5U,
};
struct rpc_create_args args = {
+ .net = &init_net,
.protocol = host->h_proto,
.address = nlm_addr(host),
.addrsize = host->h_addrlen,
diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c
index e301546..e0c9189 100644
--- a/fs/lockd/mon.c
+++ b/fs/lockd/mon.c
@@ -69,6 +69,7 @@ static struct rpc_clnt *nsm_create(void)
.sin_addr.s_addr = htonl(INADDR_LOOPBACK),
};
struct rpc_create_args args = {
+ .net = &init_net,
.protocol = XPRT_TRANSPORT_UDP,
.address = (struct sockaddr *)&sin,
.addrsize = sizeof(sin),
diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index e734072..351b711 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -601,6 +601,7 @@ static int nfs_create_rpc_client(struct nfs_client *clp,
{
struct rpc_clnt *clnt = NULL;
struct rpc_create_args args = {
+ .net = &init_net,
.protocol = clp->cl_proto,
.address = (struct sockaddr *)&clp->cl_addr,
.addrsize = clp->cl_addrlen,
diff --git a/fs/nfs/mount_clnt.c b/fs/nfs/mount_clnt.c
index 59047f8..4b47203 100644
--- a/fs/nfs/mount_clnt.c
+++ b/fs/nfs/mount_clnt.c
@@ -153,6 +153,7 @@ int nfs_mount(struct nfs_mount_request *info)
.rpc_resp = &result,
};
struct rpc_create_args args = {
+ .net = &init_net,
.protocol = info->protocol,
.address = info->sap,
.addrsize = info->salen,
@@ -224,6 +225,7 @@ void nfs_umount(const struct nfs_mount_request *info)
.to_retries = 2,
};
struct rpc_create_args args = {
+ .net = &init_net,
.protocol = IPPROTO_UDP,
.address = info->sap,
.addrsize = info->salen,
diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 014482c..1112f45 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -479,6 +479,7 @@ int setup_callback_client(struct nfs4_client *clp, struct nfs4_cb_conn *cb)
.to_retries = 0,
};
struct rpc_create_args args = {
+ .net = &init_net,
.protocol = XPRT_TRANSPORT_TCP,
.address = (struct sockaddr *) &cb->cb_addr,
.addrsize = cb->cb_addrlen,
diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index 85f38a6..58c4473 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -102,6 +102,7 @@ struct rpc_procinfo {
#ifdef __KERNEL__

struct rpc_create_args {
+ struct net *net;
int protocol;
struct sockaddr *address;
size_t addrsize;
diff --git a/net/sunrpc/rpcb_clnt.c b/net/sunrpc/rpcb_clnt.c
index dac219a..83af38d 100644
--- a/net/sunrpc/rpcb_clnt.c
+++ b/net/sunrpc/rpcb_clnt.c
@@ -177,6 +177,7 @@ static DEFINE_MUTEX(rpcb_create_local_mutex);
static int rpcb_create_local(void)
{
struct rpc_create_args args = {
+ .net = &init_net,
.protocol = XPRT_TRANSPORT_TCP,
.address = (struct sockaddr *)&rpcb_inaddr_loopback,
.addrsize = sizeof(rpcb_inaddr_loopback),
@@ -228,6 +229,7 @@ static struct rpc_clnt *rpcb_create(char *hostname, struct sockaddr *srvaddr,
size_t salen, int proto, u32 version)
{
struct rpc_create_args args = {
+ .net = &init_net,
.protocol = proto,
.address = srvaddr,
.addrsize = salen,
--
1.5.5.6


2010-09-28 15:18:34

by Pavel Emelyanov

[permalink] [raw]
Subject: [PATCH 7/7] sunrpc: Tag rpc_xprt with net

The net is known from the xprt_create and this tagging will also
give un the context in the conntection workers where real sockets
are created.

Signed-off-by: Pavel Emelyanov <[email protected]>
---
include/linux/sunrpc/xprt.h | 3 ++-
net/sunrpc/xprtrdma/transport.c | 2 +-
net/sunrpc/xprtsock.c | 6 ++++--
3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
index c4f9315..89d10d2 100644
--- a/include/linux/sunrpc/xprt.h
+++ b/include/linux/sunrpc/xprt.h
@@ -224,6 +224,7 @@ struct rpc_xprt {
bklog_u; /* backlog queue utilization */
} stat;

+ struct net *xprt_net;
const char *address_strings[RPC_DISPLAY_MAX];
};

@@ -281,7 +282,7 @@ void xprt_release_xprt_cong(struct rpc_xprt *xprt, struct rpc_task *task);
void xprt_release(struct rpc_task *task);
struct rpc_xprt * xprt_get(struct rpc_xprt *xprt);
void xprt_put(struct rpc_xprt *xprt);
-struct rpc_xprt * xprt_alloc(int size, int max_req);
+struct rpc_xprt * xprt_alloc(struct net *net, int size, int max_req);
void xprt_free(struct rpc_xprt *);

static inline __be32 *xprt_skip_transport_header(struct rpc_xprt *xprt, __be32 *p)
diff --git a/net/sunrpc/xprtrdma/transport.c b/net/sunrpc/xprtrdma/transport.c
index 0f7a1b9..2da32b4 100644
--- a/net/sunrpc/xprtrdma/transport.c
+++ b/net/sunrpc/xprtrdma/transport.c
@@ -283,7 +283,7 @@ xprt_setup_rdma(struct xprt_create *args)
return ERR_PTR(-EBADF);
}

- xprt = xprt_alloc(sizeof(struct rpcrdma_xprt),
+ xprt = xprt_alloc(args->net, sizeof(struct rpcrdma_xprt),
xprt_rdma_slot_table_entries);
if (xprt == NULL) {
dprintk("RPC: %s: couldn't allocate rpcrdma_xprt\n",
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 77cb595..dbac32e 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -759,7 +759,7 @@ static void xs_tcp_close(struct rpc_xprt *xprt)
xs_tcp_shutdown(xprt);
}

-struct rpc_xprt *xprt_alloc(int size, int max_req)
+struct rpc_xprt *xprt_alloc(struct net *net, int size, int max_req)
{
struct rpc_xprt *xprt;

@@ -772,6 +772,7 @@ struct rpc_xprt *xprt_alloc(int size, int max_req)
if (xprt->slot == NULL)
goto out_free;

+ xprt->xprt_net = get_net(net);
return xprt;

out_free:
@@ -783,6 +784,7 @@ EXPORT_SYMBOL_GPL(xprt_alloc);

void xprt_free(struct rpc_xprt *xprt)
{
+ put_net(xprt->xprt_net);
kfree(xprt->slot);
kfree(xprt);
}
@@ -2301,7 +2303,7 @@ static struct rpc_xprt *xs_setup_xprt(struct xprt_create *args,
return ERR_PTR(-EBADF);
}

- xprt = xprt_alloc(sizeof(*new), slot_table_size);
+ xprt = xprt_alloc(args->net, sizeof(*new), slot_table_size);
if (xprt == NULL) {
dprintk("RPC: xs_setup_xprt: couldn't allocate "
"rpc_xprt\n");
--
1.5.5.6


2010-09-28 15:35:19

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 1/7] sunrpc: Factor out rpc_xprt allocation


On Sep 28, 2010, at 11:15 AM, Pavel Emelyanov wrote:

> Signed-off-by: Pavel Emelyanov <[email protected]>
>
> ---
> include/linux/sunrpc/xprt.h | 1 +
> net/sunrpc/xprtrdma/transport.c | 13 ++-----------
> net/sunrpc/xprtsock.c | 37 +++++++++++++++++++++++++------------
> 3 files changed, 28 insertions(+), 23 deletions(-)
>
> diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h
> index ff5a77b..00f6e3f 100644
> --- a/include/linux/sunrpc/xprt.h
> +++ b/include/linux/sunrpc/xprt.h
> @@ -280,6 +280,7 @@ void xprt_release_xprt_cong(struct rpc_xprt *xprt, struct rpc_task *task);
> void xprt_release(struct rpc_task *task);
> struct rpc_xprt * xprt_get(struct rpc_xprt *xprt);
> void xprt_put(struct rpc_xprt *xprt);
> +struct rpc_xprt * xprt_alloc(int size, int max_req);
>
> static inline __be32 *xprt_skip_transport_header(struct rpc_xprt *xprt, __be32 *p)
> {
> diff --git a/net/sunrpc/xprtrdma/transport.c b/net/sunrpc/xprtrdma/transport.c
> index a85e866..9d77bf2 100644
> --- a/net/sunrpc/xprtrdma/transport.c
> +++ b/net/sunrpc/xprtrdma/transport.c
> @@ -285,23 +285,14 @@ xprt_setup_rdma(struct xprt_create *args)
> return ERR_PTR(-EBADF);
> }
>
> - xprt = kzalloc(sizeof(struct rpcrdma_xprt), GFP_KERNEL);
> + xprt = xprt_alloc(sizeof(struct rpcrdma_xprt),
> + xprt_rdma_slot_table_entries);
> if (xprt == NULL) {
> dprintk("RPC: %s: couldn't allocate rpcrdma_xprt\n",
> __func__);
> return ERR_PTR(-ENOMEM);
> }
>
> - xprt->max_reqs = xprt_rdma_slot_table_entries;
> - xprt->slot = kcalloc(xprt->max_reqs,
> - sizeof(struct rpc_rqst), GFP_KERNEL);
> - if (xprt->slot == NULL) {
> - dprintk("RPC: %s: couldn't allocate %d slots\n",
> - __func__, xprt->max_reqs);
> - kfree(xprt);
> - return ERR_PTR(-ENOMEM);
> - }
> -
> /* 60 second timeout, no retries */
> xprt->timeout = &xprt_rdma_default_timeout;
> xprt->bind_timeout = (60U * HZ);
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index b6309db..9f2dd3b 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -759,6 +759,28 @@ static void xs_tcp_close(struct rpc_xprt *xprt)
> xs_tcp_shutdown(xprt);
> }
>
> +struct rpc_xprt *xprt_alloc(int size, int max_req)
> +{
> + struct rpc_xprt *xprt;
> +
> + xprt = kzalloc(size, GFP_KERNEL);
> + if (xprt == NULL)
> + goto out;
> +
> + xprt->max_reqs = max_req;
> + xprt->slot = kcalloc(max_req, sizeof(struct rpc_rqst), GFP_KERNEL);
> + if (xprt->slot == NULL)
> + goto out_free;
> +
> + return xprt;
> +
> +out_free:
> + kfree(xprt);
> +out:
> + return NULL;
> +}
> +EXPORT_SYMBOL_GPL(xprt_alloc);

If xprt_alloc is a generic function, used by different transport capabilities, then it belongs in net/sunrpc/xprt.c, not in a transport-specific source file like xprtsock.c .

Also, would it makes sense to allocate these via a single kcalloc request, or would that possibly result in a high-order (ie non-zero) allocation in some cases?

> +
> /**
> * xs_destroy - prepare to shutdown a transport
> * @xprt: doomed transport
> @@ -2273,23 +2295,14 @@ static struct rpc_xprt *xs_setup_xprt(struct xprt_create *args,
> return ERR_PTR(-EBADF);
> }
>
> - new = kzalloc(sizeof(*new), GFP_KERNEL);
> - if (new == NULL) {
> + xprt = xprt_alloc(sizeof(*new), slot_table_size);
> + if (xprt == NULL) {
> dprintk("RPC: xs_setup_xprt: couldn't allocate "
> "rpc_xprt\n");
> return ERR_PTR(-ENOMEM);
> }
> - xprt = &new->xprt;
> -
> - xprt->max_reqs = slot_table_size;
> - xprt->slot = kcalloc(xprt->max_reqs, sizeof(struct rpc_rqst), GFP_KERNEL);
> - if (xprt->slot == NULL) {
> - kfree(xprt);
> - dprintk("RPC: xs_setup_xprt: couldn't allocate slot "
> - "table\n");
> - return ERR_PTR(-ENOMEM);
> - }
>
> + new = container_of(xprt, struct sock_xprt, xprt);
> memcpy(&xprt->addr, args->dstaddr, args->addrlen);
> xprt->addrlen = args->addrlen;
> if (args->srcaddr)
> --
> 1.5.5.6
>

--
chuck[dot]lever[at]oracle[dot]com





2010-09-28 15:16:25

by Pavel Emelyanov

[permalink] [raw]
Subject: [PATCH 3/7] sunrpc: Add net argument to svc_create_xprt

Signed-off-by: Pavel Emelyanov <[email protected]>
---
fs/lockd/svc.c | 2 +-
fs/nfs/callback.c | 4 ++--
fs/nfsd/nfsctl.c | 4 ++--
fs/nfsd/nfssvc.c | 5 +++--
include/linux/sunrpc/svc_xprt.h | 4 ++--
net/sunrpc/svc_xprt.c | 4 ++--
6 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index f1bacf1..b13aabc 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -206,7 +206,7 @@ static int create_lockd_listener(struct svc_serv *serv, const char *name,

xprt = svc_find_xprt(serv, name, family, 0);
if (xprt == NULL)
- return svc_create_xprt(serv, name, family, port,
+ return svc_create_xprt(serv, name, &init_net, family, port,
SVC_SOCK_DEFAULTS);
svc_xprt_put(xprt);
return 0;
diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
index e17b49e..aeec017 100644
--- a/fs/nfs/callback.c
+++ b/fs/nfs/callback.c
@@ -109,7 +109,7 @@ nfs4_callback_up(struct svc_serv *serv)
{
int ret;

- ret = svc_create_xprt(serv, "tcp", PF_INET,
+ ret = svc_create_xprt(serv, "tcp", &init_net, PF_INET,
nfs_callback_set_tcpport, SVC_SOCK_ANONYMOUS);
if (ret <= 0)
goto out_err;
@@ -117,7 +117,7 @@ nfs4_callback_up(struct svc_serv *serv)
dprintk("NFS: Callback listener port = %u (af %u)\n",
nfs_callback_tcpport, PF_INET);

- ret = svc_create_xprt(serv, "tcp", PF_INET6,
+ ret = svc_create_xprt(serv, "tcp", &init_net, PF_INET6,
nfs_callback_set_tcpport, SVC_SOCK_ANONYMOUS);
if (ret > 0) {
nfs_callback_tcpport6 = ret;
diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index b6e192d..b81da24 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1015,12 +1015,12 @@ static ssize_t __write_ports_addxprt(char *buf)
if (err != 0)
return err;

- err = svc_create_xprt(nfsd_serv, transport,
+ err = svc_create_xprt(nfsd_serv, transport, &init_net,
PF_INET, port, SVC_SOCK_ANONYMOUS);
if (err < 0)
goto out_err;

- err = svc_create_xprt(nfsd_serv, transport,
+ err = svc_create_xprt(nfsd_serv, transport, &init_net,
PF_INET6, port, SVC_SOCK_ANONYMOUS);
if (err < 0 && err != -EAFNOSUPPORT)
goto out_close;
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index e2c4346..2bae1d8 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -16,6 +16,7 @@
#include <linux/lockd/bind.h>
#include <linux/nfsacl.h>
#include <linux/seq_file.h>
+#include <net/net_namespace.h>
#include "nfsd.h"
#include "cache.h"
#include "vfs.h"
@@ -186,12 +187,12 @@ static int nfsd_init_socks(int port)
if (!list_empty(&nfsd_serv->sv_permsocks))
return 0;

- error = svc_create_xprt(nfsd_serv, "udp", PF_INET, port,
+ error = svc_create_xprt(nfsd_serv, "udp", &init_net, PF_INET, port,
SVC_SOCK_DEFAULTS);
if (error < 0)
return error;

- error = svc_create_xprt(nfsd_serv, "tcp", PF_INET, port,
+ error = svc_create_xprt(nfsd_serv, "tcp", &init_net, PF_INET, port,
SVC_SOCK_DEFAULTS);
if (error < 0)
return error;
diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index e50e3ec..646263c 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -74,8 +74,8 @@ int svc_reg_xprt_class(struct svc_xprt_class *);
void svc_unreg_xprt_class(struct svc_xprt_class *);
void svc_xprt_init(struct svc_xprt_class *, struct svc_xprt *,
struct svc_serv *);
-int svc_create_xprt(struct svc_serv *, const char *, const int,
- const unsigned short, int);
+int svc_create_xprt(struct svc_serv *, const char *, struct net *,
+ const int, const unsigned short, int);
void svc_xprt_enqueue(struct svc_xprt *xprt);
void svc_xprt_received(struct svc_xprt *);
void svc_xprt_put(struct svc_xprt *xprt);
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index f7e8915..d80789a 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -204,8 +204,8 @@ static struct svc_xprt *__svc_xpo_create(struct svc_xprt_class *xcl,
}

int svc_create_xprt(struct svc_serv *serv, const char *xprt_name,
- const int family, const unsigned short port,
- int flags)
+ struct net *net, const int family,
+ const unsigned short port, int flags)
{
struct svc_xprt_class *xcl;

--
1.5.5.6


2010-09-28 15:16:55

by Pavel Emelyanov

[permalink] [raw]
Subject: [PATCH 4/7] sunrpc: Pull net argument downto svc_create_socket

After this the socket creation in it knows the context.

Signed-off-by: Pavel Emelyanov <[email protected]>
---
include/linux/sunrpc/svc_xprt.h | 1 +
net/sunrpc/svc_xprt.c | 5 +++--
net/sunrpc/svcsock.c | 10 +++++++---
net/sunrpc/xprtrdma/svc_rdma_transport.c | 2 ++
4 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index 646263c..bb18297 100644
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -12,6 +12,7 @@

struct svc_xprt_ops {
struct svc_xprt *(*xpo_create)(struct svc_serv *,
+ struct net *net,
struct sockaddr *, int,
int);
struct svc_xprt *(*xpo_accept)(struct svc_xprt *);
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index d80789a..678b6ee 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -166,6 +166,7 @@ EXPORT_SYMBOL_GPL(svc_xprt_init);

static struct svc_xprt *__svc_xpo_create(struct svc_xprt_class *xcl,
struct svc_serv *serv,
+ struct net *net,
const int family,
const unsigned short port,
int flags)
@@ -200,7 +201,7 @@ static struct svc_xprt *__svc_xpo_create(struct svc_xprt_class *xcl,
return ERR_PTR(-EAFNOSUPPORT);
}

- return xcl->xcl_ops->xpo_create(serv, sap, len, flags);
+ return xcl->xcl_ops->xpo_create(serv, net, sap, len, flags);
}

int svc_create_xprt(struct svc_serv *serv, const char *xprt_name,
@@ -221,7 +222,7 @@ int svc_create_xprt(struct svc_serv *serv, const char *xprt_name,
goto err;

spin_unlock(&svc_xprt_class_lock);
- newxprt = __svc_xpo_create(xcl, serv, family, port, flags);
+ newxprt = __svc_xpo_create(xcl, serv, net, family, port, flags);
if (IS_ERR(newxprt)) {
module_put(xcl->xcl_owner);
return PTR_ERR(newxprt);
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 7e534dd..5593385 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -64,7 +64,8 @@ static void svc_tcp_sock_detach(struct svc_xprt *);
static void svc_sock_free(struct svc_xprt *);

static struct svc_xprt *svc_create_socket(struct svc_serv *, int,
- struct sockaddr *, int, int);
+ struct net *, struct sockaddr *,
+ int, int);
#ifdef CONFIG_DEBUG_LOCK_ALLOC
static struct lock_class_key svc_key[2];
static struct lock_class_key svc_slock_key[2];
@@ -657,10 +658,11 @@ static struct svc_xprt *svc_udp_accept(struct svc_xprt *xprt)
}

static struct svc_xprt *svc_udp_create(struct svc_serv *serv,
+ struct net *net,
struct sockaddr *sa, int salen,
int flags)
{
- return svc_create_socket(serv, IPPROTO_UDP, sa, salen, flags);
+ return svc_create_socket(serv, IPPROTO_UDP, net, sa, salen, flags);
}

static struct svc_xprt_ops svc_udp_ops = {
@@ -1178,10 +1180,11 @@ static int svc_tcp_has_wspace(struct svc_xprt *xprt)
}

static struct svc_xprt *svc_tcp_create(struct svc_serv *serv,
+ struct net *net,
struct sockaddr *sa, int salen,
int flags)
{
- return svc_create_socket(serv, IPPROTO_TCP, sa, salen, flags);
+ return svc_create_socket(serv, IPPROTO_TCP, net, sa, salen, flags);
}

static struct svc_xprt_ops svc_tcp_ops = {
@@ -1385,6 +1388,7 @@ EXPORT_SYMBOL_GPL(svc_addsock);
*/
static struct svc_xprt *svc_create_socket(struct svc_serv *serv,
int protocol,
+ struct net *net,
struct sockaddr *sin, int len,
int flags)
{
diff --git a/net/sunrpc/xprtrdma/svc_rdma_transport.c b/net/sunrpc/xprtrdma/svc_rdma_transport.c
index edea15a..950a206 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_transport.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_transport.c
@@ -52,6 +52,7 @@
#define RPCDBG_FACILITY RPCDBG_SVCXPRT

static struct svc_xprt *svc_rdma_create(struct svc_serv *serv,
+ struct net *net,
struct sockaddr *sa, int salen,
int flags);
static struct svc_xprt *svc_rdma_accept(struct svc_xprt *xprt);
@@ -670,6 +671,7 @@ static int rdma_cma_handler(struct rdma_cm_id *cma_id,
* Create a listening RDMA service endpoint.
*/
static struct svc_xprt *svc_rdma_create(struct svc_serv *serv,
+ struct net *net,
struct sockaddr *sa, int salen,
int flags)
{
--
1.5.5.6


2010-09-28 16:51:35

by Pavel Emelyanov

[permalink] [raw]
Subject: Re: [PATCH 1/7] sunrpc: Factor out rpc_xprt allocation

>> +struct rpc_xprt *xprt_alloc(int size, int max_req)
>> +{
>> + struct rpc_xprt *xprt;
>> +
>> + xprt = kzalloc(size, GFP_KERNEL);
>> + if (xprt == NULL)
>> + goto out;
>> +
>> + xprt->max_reqs = max_req;
>> + xprt->slot = kcalloc(max_req, sizeof(struct rpc_rqst), GFP_KERNEL);
>> + if (xprt->slot == NULL)
>> + goto out_free;
>> +
>> + return xprt;
>> +
>> +out_free:
>> + kfree(xprt);
>> +out:
>> + return NULL;
>> +}
>> +EXPORT_SYMBOL_GPL(xprt_alloc);
>
> If xprt_alloc is a generic function, used by different transport capabilities,
> then it belongs in net/sunrpc/xprt.c, not in a transport-specific source file
> like xprtsock.c .

Will do.

> Also, would it makes sense to allocate these via a single kcalloc request, or
> would that possibly result in a high-order (ie non-zero) allocation in some cases?

The kcalloc is used to allocate n elements of an equal size, but the first
element in this hypothetical allocation will be of another size...

2010-09-28 17:04:15

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 1/7] sunrpc: Factor out rpc_xprt allocation


On Sep 28, 2010, at 12:51 PM, Pavel Emelyanov wrote:

>>> +struct rpc_xprt *xprt_alloc(int size, int max_req)
>>> +{
>>> + struct rpc_xprt *xprt;
>>> +
>>> + xprt = kzalloc(size, GFP_KERNEL);
>>> + if (xprt == NULL)
>>> + goto out;
>>> +
>>> + xprt->max_reqs = max_req;
>>> + xprt->slot = kcalloc(max_req, sizeof(struct rpc_rqst), GFP_KERNEL);
>>> + if (xprt->slot == NULL)
>>> + goto out_free;
>>> +
>>> + return xprt;
>>> +
>>> +out_free:
>>> + kfree(xprt);
>>> +out:
>>> + return NULL;
>>> +}
>>> +EXPORT_SYMBOL_GPL(xprt_alloc);
>>
>> If xprt_alloc is a generic function, used by different transport capabilities,
>> then it belongs in net/sunrpc/xprt.c, not in a transport-specific source file
>> like xprtsock.c .
>
> Will do.
>
>> Also, would it makes sense to allocate these via a single kcalloc request, or
>> would that possibly result in a high-order (ie non-zero) allocation in some cases?
>
> The kcalloc is used to allocate n elements of an equal size, but the first
> element in this hypothetical allocation will be of another size...

You can compute the needed total allocation size by hand, and then use kzalloc. Just a thought.

Again, not a requirement, but might be an interesting optimization.

--
chuck[dot]lever[at]oracle[dot]com