Re: [RFC,PATCH 12/38] svc: Add a generic transport svc_create_xprt function

["J. Bruce Fields" <bfields@fieldses.org>]

On Fri, Dec 21, 2007 at 12:18:37PM -0600, Tom Tucker wrote:
> 
> On Fri, 2007-12-14 at 18:05 -0500, J. Bruce Fields wrote:
> > On Thu, Nov 29, 2007 at 04:56:03PM -0600, Tom Tucker wrote:
> > > --- a/net/sunrpc/svc_xprt.c
> > > +++ b/net/sunrpc/svc_xprt.c
> > > @@ -93,3 +93,40 @@ void svc_xprt_init(struct svc_xprt_class *xcl, struct svc_xprt *xprt)
> > >  	xprt->xpt_ops = xcl->xcl_ops;
> > >  }
> > >  EXPORT_SYMBOL_GPL(svc_xprt_init);
> > > +
> > > +int svc_create_xprt(struct svc_serv *serv, char *xprt_name, unsigned short port,
> > > +		    int flags)
> > > +{
> > > +	struct svc_xprt_class *xcl;
> > > +	int ret = -ENOENT;
> > > +	struct sockaddr_in sin = {
> > > +		.sin_family		= AF_INET,
> > > +		.sin_addr.s_addr	= INADDR_ANY,
> > > +		.sin_port		= htons(port),
> > > +	};
> > > +	dprintk("svc: creating transport %s[%d]\n", xprt_name, port);
> > > +	spin_lock(&svc_xprt_class_lock);
> > > +	list_for_each_entry(xcl, &svc_xprt_class_list, xcl_list) {
> > > +		if (strcmp(xprt_name, xcl->xcl_name) == 0) {
> > > +			spin_unlock(&svc_xprt_class_lock);
> > > +			if (try_module_get(xcl->xcl_owner)) {
> > 
> > 
> > Hm.  I wonder if this is right.  Don't you need to do the try_module_get
> > while still under the svc_xprt_class_lock?
> > 
> 
> Hmm. This is an interesting question. I'm was assuming that the
> unregister call from the transport provider comes from the module_exit
> function.

Oh.  Are all module init/exit functions serialized?

The case I was wondering about was something like:

	task 1				task 2
	registering xcl named "foo"	unregistering different xcl also
					named "foo".

	Gets pointer to xcl which
	task 2 is unregistering,
	drops svc_xprt_class_lock.

					acquires svc_xprt_class_lock,
					frees the xcl.

	try_module_get(xcl->cl_owner)

But I don't understand the module code well enough to understand whether
that's possible.

> If it doesn't then you are correct you could have an active
> reference to a module that is not present in the transport class list. 

I think it's fine to assume that register will always be called from
init code and unregister from the module code, so if that solves the
problem, great.

--b.