All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chuck Lever <chuck.lever@oracle.com>
To: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: linux-nfs@vger.kernel.org
Subject: Re: [PATCH 03/11] lockd: Add nlm_alloc_host()
Date: Thu, 9 Dec 2010 12:06:49 -0500	[thread overview]
Message-ID: <DA51E3C7-7F41-417E-99A2-B7DD58B1962F@oracle.com> (raw)
In-Reply-To: <1291913887.2931.1.camel@heimdal.trondhjem.org>


On Dec 9, 2010, at 11:58 AM, Trond Myklebust wrote:

> On Thu, 2010-12-09 at 11:48 -0500, Chuck Lever wrote:
>> Refactor nlm_host allocation and initialization to a separate function.
>> This will be the common piece of server and client nlm_host lookup
>> logic after the nlm_host cache is split.
>> 
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>> 
>> fs/lockd/host.c |  106 ++++++++++++++++++++++++++++++++-----------------------
>> 1 files changed, 61 insertions(+), 45 deletions(-)
>> 
>> diff --git a/fs/lockd/host.c b/fs/lockd/host.c
>> index 2dbf139..ed1895a 100644
>> --- a/fs/lockd/host.c
>> +++ b/fs/lockd/host.c
>> @@ -100,6 +100,64 @@ static unsigned int nlm_hash_address(const struct sockaddr *sap)
>> }
>> 
>> /*
>> + * Allocate and initialize an nlm_host.  Common to both client and server.
>> + */
>> +static struct nlm_host *nlm_alloc_host(struct nlm_lookup_host_info *ni,
>> +				       struct nsm_handle *nsm)
>> +{
>> +	struct nlm_host *host = NULL;
>> +	unsigned long now = jiffies;
>> +
>> +	if (nsm != NULL)
>> +		atomic_inc(&nsm->sm_count);
>> +	else {
>> +		host = NULL;
>> +		nsm = nsm_get_handle(ni->sap, ni->salen,
>> +					ni->hostname, ni->hostname_len);
>> +		if (unlikely(nsm == NULL)) {
>> +			dprintk("lockd: %s failed; no nsm handle\n",
>> +				__func__);
>> +			goto out;
>> +		}
>> +	}
>> +
>> +	host = kzalloc(sizeof(*host), GFP_KERNEL);
>> +	if (unlikely(host == NULL)) {
>> +		dprintk("lockd: %s failed; no memory\n", __func__);
>> +		nsm_release(nsm);
>> +		goto out;
>> +	}
>> +
>> +	memcpy(nlm_addr(host), ni->sap, ni->salen);
>> +	host->h_addrlen    = ni->salen;
>> +	rpc_set_port(nlm_addr(host), 0);
>> +
>> +	host->h_rpcclnt    = NULL;
> 
> OK. I'm at a complete loss to explain why you would want to kzalloc()
> the storage, and then afterwards explicitly initialise these fields to
> zero.

This copies the old code, which also does this.  See below.

> Please choose to do one or the other, but not both...

Since we are initializing most of the fields anyway, I think we can do without kzalloc().

>> +	host->h_name	   = nsm->sm_name;
>> +	host->h_version    = ni->version;
>> +	host->h_proto      = ni->protocol;
>> +	host->h_server     = ni->server;
>> +	host->h_noresvport = ni->noresvport;
>> +	init_waitqueue_head(&host->h_gracewait);
>> +	init_rwsem(&host->h_rwsem);
>> +	host->h_state      = 0;
>> +	host->h_nsmstate   = 0;
>> +	atomic_set(&host->h_count, 1);
>> +	mutex_init(&host->h_mutex);
>> +	host->h_nextrebind = now + NLM_HOST_REBIND;
>> +	host->h_expires    = now + NLM_HOST_EXPIRE;
>> +	INIT_LIST_HEAD(&host->h_lockowners);
>> +	spin_lock_init(&host->h_lock);
>> +	INIT_LIST_HEAD(&host->h_granted);
>> +	INIT_LIST_HEAD(&host->h_reclaim);
>> +	host->h_nsmhandle  = nsm;
>> +	host->h_addrbuf    = nsm->sm_addrbuf;
>> +
>> +out:
>> +	return host;
>> +}
>> +
>> +/*
>>  * Common host lookup routine for server & client
>>  */
>> static struct nlm_host *nlm_lookup_host(struct nlm_lookup_host_info *ni)
>> @@ -150,55 +208,13 @@ static struct nlm_host *nlm_lookup_host(struct nlm_lookup_host_info *ni)
>> 		goto out;
>> 	}
>> 
>> -	/*
>> -	 * The host wasn't in our hash table.  If we don't
>> -	 * have an NSM handle for it yet, create one.
>> -	 */
>> -	if (nsm)
>> -		atomic_inc(&nsm->sm_count);
>> -	else {
>> -		host = NULL;
>> -		nsm = nsm_get_handle(ni->sap, ni->salen,
>> -					ni->hostname, ni->hostname_len);
>> -		if (!nsm) {
>> -			dprintk("lockd: nlm_lookup_host failed; "
>> -				"no nsm handle\n");
>> -			goto out;
>> -		}
>> -	}
>> -
>> -	host = kzalloc(sizeof(*host), GFP_KERNEL);
>> -	if (!host) {
>> -		nsm_release(nsm);
>> -		dprintk("lockd: nlm_lookup_host failed; no memory\n");
>> +	host = nlm_alloc_host(ni, nsm);
>> +	if (unlikely(host == NULL))
>> 		goto out;
>> -	}
>> -	host->h_name	   = nsm->sm_name;
>> -	host->h_addrbuf    = nsm->sm_addrbuf;
>> -	memcpy(nlm_addr(host), ni->sap, ni->salen);
>> -	host->h_addrlen = ni->salen;
>> -	rpc_set_port(nlm_addr(host), 0);
>> +
>> 	memcpy(nlm_srcaddr(host), ni->src_sap, ni->src_len);
>> 	host->h_srcaddrlen = ni->src_len;
>> -	host->h_version    = ni->version;
>> -	host->h_proto      = ni->protocol;
>> -	host->h_rpcclnt    = NULL;
>> -	mutex_init(&host->h_mutex);
>> -	host->h_nextrebind = jiffies + NLM_HOST_REBIND;
>> -	host->h_expires    = jiffies + NLM_HOST_EXPIRE;
>> -	atomic_set(&host->h_count, 1);
>> -	init_waitqueue_head(&host->h_gracewait);
>> -	init_rwsem(&host->h_rwsem);
>> -	host->h_state      = 0;			/* pseudo NSM state */
>> -	host->h_nsmstate   = 0;			/* real NSM state */
>> -	host->h_nsmhandle  = nsm;
>> -	host->h_server	   = ni->server;
>> -	host->h_noresvport = ni->noresvport;
>> 	hlist_add_head(&host->h_hash, chain);
>> -	INIT_LIST_HEAD(&host->h_lockowners);
>> -	spin_lock_init(&host->h_lock);
>> -	INIT_LIST_HEAD(&host->h_granted);
>> -	INIT_LIST_HEAD(&host->h_reclaim);
>> 
>> 	nrhosts++;
>> 
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> -- 
> Trond Myklebust
> Linux NFS client maintainer
> 
> NetApp
> Trond.Myklebust@netapp.com
> www.netapp.com
> 

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





  reply	other threads:[~2010-12-09 17:07 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-09 16:48 [PATCH 00/11] RFC: Split nlm_host cache Chuck Lever
2010-12-09 16:48 ` [PATCH 01/11] lockd: define host_for_each{_safe} macros Chuck Lever
2010-12-09 16:48 ` [PATCH 02/11] lockd: reorganize nlm_host_rebooted Chuck Lever
2010-12-09 16:48 ` [PATCH 03/11] lockd: Add nlm_alloc_host() Chuck Lever
2010-12-09 16:58   ` Trond Myklebust
2010-12-09 17:06     ` Chuck Lever [this message]
2010-12-09 16:48 ` [PATCH 04/11] lockd: Add nlm_destroy_host_locked() Chuck Lever
2010-12-09 16:48 ` [PATCH 05/11] lockd: Split nlm_release_call() Chuck Lever
2010-12-09 16:49 ` [PATCH 06/11] lockd: Create client-side nlm_host cache Chuck Lever
2010-12-09 16:49 ` [PATCH 07/11] lockd: Clean up nlmsvc_lookup_host() Chuck Lever
2010-12-09 16:49 ` [PATCH 08/11] lockd: Rename nlm_hosts Chuck Lever
2010-12-09 16:50 ` [PATCH 09/11] lockd: Make nrhosts an unsigned long Chuck Lever
2010-12-09 16:50 ` [PATCH 10/11] lockd: Remove nlm_lookup_host() Chuck Lever
2010-12-09 16:50 ` [PATCH 11/11] lockd: Remove src_sap and src_len from nlm_lookup_host_info struct Chuck Lever
2010-12-14 15:04 [PATCH 00/11] nlm_host cache split for 2.6.38 Chuck Lever
2010-12-14 15:05 ` [PATCH 03/11] lockd: Add nlm_alloc_host() Chuck Lever

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DA51E3C7-7F41-417E-99A2-B7DD58B1962F@oracle.com \
    --to=chuck.lever@oracle.com \
    --cc=Trond.Myklebust@netapp.com \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.