2005-10-06 14:13:14

by Lever, Charles

[permalink] [raw]
Subject: RE: [NFS] [RFC: 2.6 patch] net/sunrpc/: possible cleanups

actually, can we hold off on this change? the RPC transport switch will
eventually need most of those EXPORT_SYMBOLs.

the only harmless change i see below is removing xdr_decode_string().

> -----Original Message-----
> From: Adrian Bunk [mailto:[email protected]]
> Sent: Saturday, October 01, 2005 10:21 AM
> To: David Miller
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]
> Subject: [NFS] [RFC: 2.6 patch] net/sunrpc/: possible cleanups
>
> This patch contains the following possible cleanups:
> - make needlessly global code static
> - #if 0 the following unused global function:
> - xdr.c: xdr_decode_string
> - remove the following unneeded EXPORT_SYMBOL's:
> - auth_gss/gss_mech_switch.c: gss_mech_get
> - auth_gss/gss_mech_switch.c: gss_mech_get_by_name
> - auth_gss/gss_mech_switch.c: gss_mech_get_by_pseudoflavor
> - auth_gss/gss_mech_switch.c: gss_pseudoflavor_to_service
> - auth_gss/gss_mech_switch.c: gss_service_to_auth_domain_name
> - auth_gss/gss_mech_switch.c: gss_mech_put
> - sunrpc_syms.c: rpc_wake_up_next
> - sunrpc_syms.c: rpc_new_child
> - sunrpc_syms.c: rpc_run_child
> - sunrpc_syms.c: rpc_new_task
> - sunrpc_syms.c: rpc_release_task
> - sunrpc_syms.c: rpc_release_client
> - sunrpc_syms.c: xprt_udp_slot_table_entries
> - sunrpc_syms.c: xprt_tcp_slot_table_entries
> - sunrpc_syms.c: svc_drop
> - sunrpc_syms.c: svc_authenticate
> - sunrpc_syms.c: xdr_decode_string
>
> Please review which of these patches do make sense and which conflict
> with pending patches.
>
> Signed-off-by: Adrian Bunk <[email protected]>
>
> ---
>
> This patch was already sent on:
> - 30 May 2005
> - 7 May 2005
>
> include/linux/sunrpc/clnt.h | 1 -
> include/linux/sunrpc/gss_api.h | 3 ---
> include/linux/sunrpc/xdr.h | 2 --
> net/sunrpc/auth_gss/gss_mech_switch.c | 13 +------------
> net/sunrpc/clnt.c | 3 ++-
> net/sunrpc/sunrpc_syms.c | 11 -----------
> net/sunrpc/xdr.c | 4 +++-
> 7 files changed, 6 insertions(+), 31 deletions(-)
>
> ---
> linux-2.6.12-rc3-mm3-full/include/linux/sunrpc/gss_api.h.old
> 2005-05-05 23:05:01.000000000 +0200
> +++ linux-2.6.12-rc3-mm3-full/include/linux/sunrpc/gss_api.h
> 2005-05-05 23:05:10.000000000 +0200
> @@ -110,9 +110,6 @@
> /* Similar, but get by pseudoflavor. */
> struct gss_api_mech *gss_mech_get_by_pseudoflavor(u32);
>
> -/* Just increments the mechanism's reference count and
> returns its input: */
> -struct gss_api_mech * gss_mech_get(struct gss_api_mech *);
> -
> /* For every succesful gss_mech_get or gss_mech_get_by_*
> call there must be a
> * corresponding call to gss_mech_put. */
> void gss_mech_put(struct gss_api_mech *);
> ---
> linux-2.6.12-rc3-mm3-full/net/sunrpc/auth_gss/gss_mech_switc
> h.c.old 2005-05-05 23:05:17.000000000 +0200
> +++
> linux-2.6.12-rc3-mm3-full/net/sunrpc/auth_gss/gss_mech_s
> witch.c 2005-05-05 23:19:33.000000000 +0200
> @@ -133,14 +133,13 @@
>
> EXPORT_SYMBOL(gss_mech_unregister);
>
> -struct gss_api_mech *
> +static struct gss_api_mech *
> gss_mech_get(struct gss_api_mech *gm)
> {
> __module_get(gm->gm_owner);
> return gm;
> }
>
> -EXPORT_SYMBOL(gss_mech_get);
>
> struct gss_api_mech *
> gss_mech_get_by_name(const char *name)
> @@ -160,8 +159,6 @@
>
> }
>
> -EXPORT_SYMBOL(gss_mech_get_by_name);
> -
> static inline int
> mech_supports_pseudoflavor(struct gss_api_mech *gm, u32 pseudoflavor)
> {
> @@ -193,8 +190,6 @@
> return gm;
> }
>
> -EXPORT_SYMBOL(gss_mech_get_by_pseudoflavor);
> -
> u32
> gss_pseudoflavor_to_service(struct gss_api_mech *gm, u32
> pseudoflavor)
> {
> @@ -207,8 +202,6 @@
> return 0;
> }
>
> -EXPORT_SYMBOL(gss_pseudoflavor_to_service);
> -
> char *
> gss_service_to_auth_domain_name(struct gss_api_mech *gm, u32 service)
> {
> @@ -221,16 +214,12 @@
> return NULL;
> }
>
> -EXPORT_SYMBOL(gss_service_to_auth_domain_name);
> -
> void
> gss_mech_put(struct gss_api_mech * gm)
> {
> module_put(gm->gm_owner);
> }
>
> -EXPORT_SYMBOL(gss_mech_put);
> -
> /* The mech could probably be determined from the token
> instead, but it's just
> * as easy for now to pass it in. */
> int
> --- linux-2.6.12-rc3-mm3-full/include/linux/sunrpc/clnt.h.old
> 2005-05-05 23:05:45.000000000 +0200
> +++ linux-2.6.12-rc3-mm3-full/include/linux/sunrpc/clnt.h
> 2005-05-05 23:05:50.000000000 +0200
> @@ -134,7 +134,6 @@
> void rpc_clnt_sigunmask(struct rpc_clnt *clnt,
> sigset_t *oldset);
> void rpc_setbufsize(struct rpc_clnt *, unsigned int,
> unsigned int);
> size_t rpc_max_payload(struct rpc_clnt *);
> -int rpc_ping(struct rpc_clnt *clnt, int flags);
>
> static __inline__
> int rpc_call(struct rpc_clnt *clnt, u32 proc, void *argp,
> void *resp, int flags)
> --- linux-2.6.12-rc3-mm3-full/net/sunrpc/clnt.c.old
> 2005-05-05 23:05:58.000000000 +0200
> +++ linux-2.6.12-rc3-mm3-full/net/sunrpc/clnt.c
> 2005-05-05 23:06:21.000000000 +0200
> @@ -63,6 +63,7 @@
> static u32 * call_header(struct rpc_task *task);
> static u32 * call_verify(struct rpc_task *task);
>
> +static int rpc_ping(struct rpc_clnt *clnt, int flags);
>
> static int
> rpc_setup_pipedir(struct rpc_clnt *clnt, char *dir_name)
> @@ -1178,7 +1179,7 @@
> .p_decode = rpcproc_decode_null,
> };
>
> -int rpc_ping(struct rpc_clnt *clnt, int flags)
> +static int rpc_ping(struct rpc_clnt *clnt, int flags)
> {
> struct rpc_message msg = {
> .rpc_proc = &rpcproc_null,
> --- linux-2.6.12-rc3-mm3-full/include/linux/sunrpc/xdr.h.old
> 2005-05-05 23:06:40.000000000 +0200
> +++ linux-2.6.12-rc3-mm3-full/include/linux/sunrpc/xdr.h
> 2005-05-05 23:07:23.000000000 +0200
> @@ -91,7 +91,6 @@
> u32 * xdr_encode_opaque_fixed(u32 *p, const void
> *ptr, unsigned int len);
> u32 * xdr_encode_opaque(u32 *p, const void *ptr,
> unsigned int len);
> u32 * xdr_encode_string(u32 *p, const char *s);
> -u32 * xdr_decode_string(u32 *p, char **sp, int *lenp,
> int maxlen);
> u32 * xdr_decode_string_inplace(u32 *p, char **sp,
> int *lenp, int maxlen);
> u32 * xdr_encode_netobj(u32 *p, const struct xdr_netobj *);
> u32 * xdr_decode_netobj(u32 *p, struct xdr_netobj *);
> @@ -147,7 +146,6 @@
> extern int xdr_buf_subsegment(struct xdr_buf *, struct
> xdr_buf *, int, int);
> extern int xdr_buf_read_netobj(struct xdr_buf *, struct
> xdr_netobj *, int);
> extern int read_bytes_from_xdr_buf(struct xdr_buf *, int,
> void *, int);
> -extern int write_bytes_to_xdr_buf(struct xdr_buf *, int,
> void *, int);
>
> /*
> * Helper structure for copying from an sk_buff.
> --- linux-2.6.12-rc3-mm3-full/net/sunrpc/xdr.c.old
> 2005-05-05 23:06:52.000000000 +0200
> +++ linux-2.6.12-rc3-mm3-full/net/sunrpc/xdr.c
> 2005-05-05 23:07:56.000000000 +0200
> @@ -95,6 +95,7 @@
> return xdr_encode_array(p, string, strlen(string));
> }
>
> +#if 0
> u32 *
> xdr_decode_string(u32 *p, char **sp, int *lenp, int maxlen)
> {
> @@ -115,6 +116,7 @@
> *sp = string;
> return p + XDR_QUADLEN(len);
> }
> +#endif /* 0 */
>
> u32 *
> xdr_decode_string_inplace(u32 *p, char **sp, int *lenp, int maxlen)
> @@ -882,7 +884,7 @@
> }
>
> /* obj is assumed to point to allocated memory of size at
> least len: */
> -int
> +static int
> write_bytes_to_xdr_buf(struct xdr_buf *buf, int base, void
> *obj, int len)
> {
> struct xdr_buf subbuf;
> --- linux-2.6.12-rc3-mm3-full/net/sunrpc/sunrpc_syms.c.old
> 2005-05-05 23:07:30.000000000 +0200
> +++ linux-2.6.12-rc3-mm3-full/net/sunrpc/sunrpc_syms.c
> 2005-05-05 23:36:43.000000000 +0200
> @@ -29,15 +29,10 @@
> EXPORT_SYMBOL(rpc_execute);
> EXPORT_SYMBOL(rpc_init_task);
> EXPORT_SYMBOL(rpc_sleep_on);
> -EXPORT_SYMBOL(rpc_wake_up_next);
> EXPORT_SYMBOL(rpc_wake_up_task);
> -EXPORT_SYMBOL(rpc_new_child);
> -EXPORT_SYMBOL(rpc_run_child);
> EXPORT_SYMBOL(rpciod_down);
> EXPORT_SYMBOL(rpciod_up);
> -EXPORT_SYMBOL(rpc_new_task);
> EXPORT_SYMBOL(rpc_wake_up_status);
> -EXPORT_SYMBOL(rpc_release_task);
>
> /* RPC client functions */
> EXPORT_SYMBOL(rpc_create_client);
> @@ -45,7 +40,6 @@
> EXPORT_SYMBOL(rpc_bind_new_program);
> EXPORT_SYMBOL(rpc_destroy_client);
> EXPORT_SYMBOL(rpc_shutdown_client);
> -EXPORT_SYMBOL(rpc_release_client);
> EXPORT_SYMBOL(rpc_killall_tasks);
> EXPORT_SYMBOL(rpc_call_sync);
> EXPORT_SYMBOL(rpc_call_async);
> @@ -63,8 +57,6 @@
> /* Client transport */
> EXPORT_SYMBOL(xprt_create_proto);
> EXPORT_SYMBOL(xprt_set_timeout);
> -EXPORT_SYMBOL(xprt_udp_slot_table_entries);
> -EXPORT_SYMBOL(xprt_tcp_slot_table_entries);
>
> /* Client credential cache */
> EXPORT_SYMBOL(rpcauth_register);
> @@ -81,7 +73,6 @@
> EXPORT_SYMBOL(svc_create_thread);
> EXPORT_SYMBOL(svc_exit_thread);
> EXPORT_SYMBOL(svc_destroy);
> -EXPORT_SYMBOL(svc_drop);
> EXPORT_SYMBOL(svc_process);
> EXPORT_SYMBOL(svc_recv);
> EXPORT_SYMBOL(svc_wake_up);
> @@ -89,7 +80,6 @@
> EXPORT_SYMBOL(svc_reserve);
> EXPORT_SYMBOL(svc_auth_register);
> EXPORT_SYMBOL(auth_domain_lookup);
> -EXPORT_SYMBOL(svc_authenticate);
> EXPORT_SYMBOL(svc_set_client);
>
> /* RPC statistics */
> @@ -122,7 +112,6 @@
>
> /* Generic XDR */
> EXPORT_SYMBOL(xdr_encode_string);
> -EXPORT_SYMBOL(xdr_decode_string);
> EXPORT_SYMBOL(xdr_decode_string_inplace);
> EXPORT_SYMBOL(xdr_decode_netobj);
> EXPORT_SYMBOL(xdr_encode_netobj);
>
>
>
> -------------------------------------------------------
> This SF.Net email is sponsored by:
> Power Architecture Resource Center: Free content, downloads,
> discussions,
> and more. http://solutions.newsforge.com/ibmarch.tmpl
> _______________________________________________
> NFS maillist - [email protected]
> https://lists.sourceforge.net/lists/listinfo/nfs
>


2005-11-23 01:24:26

by Adrian Bunk

[permalink] [raw]
Subject: [2.6 patch] net/sunrpc/xdr.c: remove xdr_decode_string()

On Thu, Oct 06, 2005 at 07:13:14AM -0700, Lever, Charles wrote:

> actually, can we hold off on this change? the RPC transport switch will
> eventually need most of those EXPORT_SYMBOLs.

Am I right to assume this will happen in the foreseeable future?

> the only harmless change i see below is removing xdr_decode_string().

Patch below.

cu
Adrian


<-- snip -->


This patch removes ths unused function xdr_decode_string().


Signed-off-by: Adrian Bunk <[email protected]>

---

include/linux/sunrpc/xdr.h | 1 -
net/sunrpc/sunrpc_syms.c | 1 -
net/sunrpc/xdr.c | 21 ---------------------
3 files changed, 23 deletions(-)

--- linux-2.6.15-rc1-mm2-full/include/linux/sunrpc/xdr.h.old 2005-11-23 02:03:01.000000000 +0100
+++ linux-2.6.15-rc1-mm2-full/include/linux/sunrpc/xdr.h 2005-11-23 02:03:08.000000000 +0100
@@ -91,7 +91,6 @@
u32 * xdr_encode_opaque_fixed(u32 *p, const void *ptr, unsigned int len);
u32 * xdr_encode_opaque(u32 *p, const void *ptr, unsigned int len);
u32 * xdr_encode_string(u32 *p, const char *s);
-u32 * xdr_decode_string(u32 *p, char **sp, int *lenp, int maxlen);
u32 * xdr_decode_string_inplace(u32 *p, char **sp, int *lenp, int maxlen);
u32 * xdr_encode_netobj(u32 *p, const struct xdr_netobj *);
u32 * xdr_decode_netobj(u32 *p, struct xdr_netobj *);
--- linux-2.6.15-rc1-mm2-full/net/sunrpc/xdr.c.old 2005-11-23 02:03:17.000000000 +0100
+++ linux-2.6.15-rc1-mm2-full/net/sunrpc/xdr.c 2005-11-23 02:03:27.000000000 +0100
@@ -93,27 +93,6 @@
}

u32 *
-xdr_decode_string(u32 *p, char **sp, int *lenp, int maxlen)
-{
- unsigned int len;
- char *string;
-
- if ((len = ntohl(*p++)) > maxlen)
- return NULL;
- if (lenp)
- *lenp = len;
- if ((len % 4) != 0) {
- string = (char *) p;
- } else {
- string = (char *) (p - 1);
- memmove(string, p, len);
- }
- string[len] = '\0';
- *sp = string;
- return p + XDR_QUADLEN(len);
-}
-
-u32 *
xdr_decode_string_inplace(u32 *p, char **sp, int *lenp, int maxlen)
{
unsigned int len;
--- linux-2.6.15-rc1-mm2-full/net/sunrpc/sunrpc_syms.c.old 2005-11-23 02:03:35.000000000 +0100
+++ linux-2.6.15-rc1-mm2-full/net/sunrpc/sunrpc_syms.c 2005-11-23 02:03:38.000000000 +0100
@@ -120,7 +120,6 @@

/* Generic XDR */
EXPORT_SYMBOL(xdr_encode_string);
-EXPORT_SYMBOL(xdr_decode_string);
EXPORT_SYMBOL(xdr_decode_string_inplace);
EXPORT_SYMBOL(xdr_decode_netobj);
EXPORT_SYMBOL(xdr_encode_netobj);



-------------------------------------------------------
This SF.Net email is sponsored by the JBoss Inc. Get Certified Today
Register for a JBoss Training Course. Free Certification Exam
for All Training Attendees Through End of 2005. For more info visit:
http://ads.osdn.com/?ad_id=7628&alloc_id=16845&op=click
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2005-11-23 12:31:38

by Lever, Charles

[permalink] [raw]
Subject: RE: [2.6 patch] net/sunrpc/xdr.c: remove xdr_decode_string()

> On Thu, Oct 06, 2005 at 07:13:14AM -0700, Lever, Charles wrote:
>=20
> > actually, can we hold off on this change? the RPC=20
> transport switch will
> > eventually need most of those EXPORT_SYMBOLs.
>=20
> Am I right to assume this will happen in the foreseeable future?

the first portion of the transport switch is in 2.6.15-rcX. at this
point i'm expecting the EXPORT_SYMBOL changes to go in 2.6.17 or later.

so i don't remember why you are removing xdr_decode_string. are we sure
that no-one will need this functionality in the future? it is harmless
to remove today, but i wonder if someone is just going to add it back
sometime.


-------------------------------------------------------
This SF.Net email is sponsored by the JBoss Inc. Get Certified Today
Register for a JBoss Training Course. Free Certification Exam
for All Training Attendees Through End of 2005. For more info visit:
http://ads.osdn.com/?ad_id=7628&alloc_id=16845&op=click
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2005-11-23 16:25:28

by Adrian Bunk

[permalink] [raw]
Subject: Re: [2.6 patch] net/sunrpc/xdr.c: remove xdr_decode_string()

On Wed, Nov 23, 2005 at 04:31:14AM -0800, Lever, Charles wrote:
> > On Thu, Oct 06, 2005 at 07:13:14AM -0700, Lever, Charles wrote:
> >
> > > actually, can we hold off on this change? the RPC
> > transport switch will
> > > eventually need most of those EXPORT_SYMBOLs.
> >
> > Am I right to assume this will happen in the foreseeable future?
>
> the first portion of the transport switch is in 2.6.15-rcX. at this
> point i'm expecting the EXPORT_SYMBOL changes to go in 2.6.17 or later.

OK.

> so i don't remember why you are removing xdr_decode_string. are we sure
> that no-one will need this functionality in the future? it is harmless
> to remove today, but i wonder if someone is just going to add it back
> sometime.

It's unused and you said:
the only harmless change i see below is removing xdr_decode_string().

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2005-11-23 23:07:31

by NeilBrown

[permalink] [raw]
Subject: Re: [2.6 patch] net/sunrpc/xdr.c: remove xdr_decode_string()

On Wednesday November 23, [email protected] wrote:
> On Wed, Nov 23, 2005 at 04:31:14AM -0800, Lever, Charles wrote:
> > so i don't remember why you are removing xdr_decode_string. are we sure
> > that no-one will need this functionality in the future? it is harmless
> > to remove today, but i wonder if someone is just going to add it back
> > sometime.
>
> It's unused and you said:
> the only harmless change i see below is removing xdr_decode_string().
>

As 'xdr_decode_string' (sometimes) modifies the buffer that it is
decoding, I don't think it's usage should be encouraged. If it is no
longer in use, then I fully support and encourage removing it.

NeilBrown


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems? Stop! Download the new AJAX search engine that makes
searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

2005-11-24 06:56:51

by Lever, Charles

[permalink] [raw]
Subject: RE: [2.6 patch] net/sunrpc/xdr.c: remove xdr_decode_string()

> On Wednesday November 23, [email protected] wrote:
> > On Wed, Nov 23, 2005 at 04:31:14AM -0800, Lever, Charles wrote:
> > > so i don't remember why you are removing
> xdr_decode_string. are we sure
> > > that no-one will need this functionality in the future?
> it is harmless
> > > to remove today, but i wonder if someone is just going to
> add it back
> > > sometime.
> >
> > It's unused and you said:
> > the only harmless change i see below is removing
> xdr_decode_string().
> >
>
> As 'xdr_decode_string' (sometimes) modifies the buffer that it is
> decoding, I don't think it's usage should be encouraged. If it is no
> longer in use, then I fully support and encourage removing it.

actually this is a good point. since it is unused, it is an untested
path as we continue to evolve the code base.

2006-04-15 21:32:15

by Adrian Bunk

[permalink] [raw]
Subject: Re: [NFS] [RFC: 2.6 patch] net/sunrpc/: possible cleanups

On Thu, Oct 06, 2005 at 07:13:14AM -0700, Lever, Charles wrote:

> actually, can we hold off on this change? the RPC transport switch will
> eventually need most of those EXPORT_SYMBOLs.
>...
> > This patch was already sent on:
> > - 30 May 2005
> > - 7 May 2005
>...

One year has passed since I sent this patch the first time, and with the
exception that it needs re-diff'ing it still applies.

Charles, are the changes you are talking about that might need them
available in the very near future?

If not, I'd suggest to remove them and re-add them when code using them
will be included in the kernel (reverting parts or all of my patch
should be trivial).

This way, they'll not continue to uselessly making the kernel image
larger.

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed


2006-04-15 23:01:06

by Lever, Charles

[permalink] [raw]
Subject: Re: [NFS] [RFC: 2.6 patch] net/sunrpc/: possible cleanups

On 4/15/06 5:32 PM, "Adrian Bunk" <[email protected]> wrote:

> On Thu, Oct 06, 2005 at 07:13:14AM -0700, Lever, Charles wrote:
>
>> actually, can we hold off on this change? the RPC transport switch will
>> eventually need most of those EXPORT_SYMBOLs.
>> ...
>>> This patch was already sent on:
>>> - 30 May 2005
>>> - 7 May 2005
>> ...
>
> One year has passed since I sent this patch the first time, and with the
> exception that it needs re-diff'ing it still applies.
>
> Charles, are the changes you are talking about that might need them
> available in the very near future?

Yes, they are coming soon.