Benny Halevy wrote:
From: Benny Halevy <bhalevy@xxxxxxxxxxx>
since commit ff7d9756b501744540be65e172d27ee321d86103
"nfsd: use static memory for callback program and stats"
do_probe_callback uses a static callback program
(NFS4_CALLBACK) rather than the one set in clp->cl_callback.cb_prog
as passed in by the client in setclientid (4.0)
or create_session (4.1).
This patches allows allocating cb_program (and cb_stats) dynamically
and setting a free_rpc_program function pointer to be
called when the rpc_clnt structure is destroyed.
Signed-off-by: Benny Halevy <bhalevy@xxxxxxxxxxx>
---
Bruce, how about the following patch instead.
It should solve the callback program number as well as Olga
gss_auth issue.
I'm still not sure about the gss_auth reference accounting
for the rpc_clnt it references, but that's somewhat orthogonal
to the solution in this patch.
Benny
fs/nfsd/nfs4callback.c | 47 ++++++++++++++++++++++++++++--------------
include/linux/sunrpc/clnt.h | 1 +
net/sunrpc/clnt.c | 2 +
3 files changed, 34 insertions(+), 16 deletions(-)
diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 30d3130..6599b1e 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -342,20 +342,11 @@ static struct rpc_version * nfs_cb_version[] = {
&nfs_cb_version4,
};
-static struct rpc_program cb_program;
-
-static struct rpc_stat cb_stats = {
- .program = &cb_program
-};
-
-#define NFS4_CALLBACK 0x40000000
-static struct rpc_program cb_program = {
- .name = "nfs4_cb",
- .number = NFS4_CALLBACK,
- .nrvers = ARRAY_SIZE(nfs_cb_version),
- .version = nfs_cb_version,
- .stats = &cb_stats,
-};
+static void free_rpc_program(struct rpc_program *cb_program)
+{
+ dprintk("%s: freeing callback program 0x%p\n", __func__, cb_program);
+ kfree(cb_program);
+}
/* Reference counting, callback cleanup, etc., all look racy as heck.
* And why is cb_set an atomic? */
@@ -371,12 +362,13 @@ static int do_probe_callback(void *data)
.to_maxval = (NFSD_LEASE_TIME/2) * HZ,
.to_exponential = 1,
};
+ struct rpc_stat *cb_stats;
+ struct rpc_program *cb_program;
struct rpc_create_args args = {
.protocol = IPPROTO_TCP,
.address = (struct sockaddr *)&addr,
.addrsize = sizeof(addr),
.timeout = &timeparms,
- .program = &cb_program,
.version = nfs_cb_version[1]->number,
.authflavor = RPC_AUTH_UNIX, /* XXX: need AUTH_GSS... */
.flags = (RPC_CLNT_CREATE_NOPING | RPC_CLNT_CREATE_QUIET),
@@ -394,8 +386,31 @@ static int do_probe_callback(void *data)
addr.sin_port = htons(cb->cb_port);
addr.sin_addr.s_addr = htonl(cb->cb_addr);
+ /* Initialize rpc_program */
+ cb_program = kzalloc(sizeof(struct rpc_program) +
+ sizeof(struct rpc_stat), GFP_KERNEL);
Ugg. What about defining a struct which contains an rpc_program
followed by an rpc_stat and then allocate and free that? This
seems a little, unclean, to me.
ps
+ if (!cb_program) {
+ dprintk("NFSD: %s: couldn't allocate callback program\n",
+ __func__);
+ status = -ENOMEM;
+ goto out_err;
+ }
+
+ cb_program->name = "nfs4_cb";
+ cb_program->number = clp->cl_callback.cb_prog;
+ cb_program->nrvers = ARRAY_SIZE(nfs_cb_version);
+ cb_program->version = nfs_cb_version;
+ cb_program->free_rpc_program = free_rpc_program;
+ args.program = cb_program;
+
+ dprintk("%s: program %s 0x%x nrvers %u version %u\n",
+ __func__, args.program->name, args.program->number,
+ args.program->nrvers, args.version);
+
/* Initialize rpc_stat */
- memset(args.program->stats, 0, sizeof(struct rpc_stat));
+ cb_stats = (struct rpc_stat *)(cb_program + 1);
+ cb_stats->program = cb_program;
+ cb_program->stats = cb_stats;
/* Create RPC client */
client = rpc_create(&args);
diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index e5bfe01..d342374 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -71,6 +71,7 @@ struct rpc_program {
struct rpc_version ** version; /* version array */
struct rpc_stat * stats; /* statistics */
char * pipe_dir_name; /* path to rpc_pipefs dir */
+ void (*free_rpc_program)(struct rpc_program *);
};
struct rpc_version {
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 76739e9..cfb21c0 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -415,6 +415,8 @@ rpc_free_client(struct kref *kref)
if (clnt->cl_server != clnt->cl_inline_name)
kfree(clnt->cl_server);
out_free:
+ if (clnt->cl_program && clnt->cl_program->free_rpc_program)
+ clnt->cl_program->free_rpc_program(clnt->cl_program);
rpc_unregister_client(clnt);
rpc_free_iostats(clnt->cl_metrics);
clnt->cl_metrics = NULL;
--
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
