On Sat, Mar 28, 2009 at 11:31:50AM +0300, Benny Halevy wrote:
> From: Marc Eshel <eshel@xxxxxxxxxxxxxxx>
>
> Simple sessionid hashing using its monotonically increasing sequence number.
>
> Locking considerations:
> sessionid_hashtbl access is controlled by the sessionid_lock spin lock.
> It must be taken for insert, delete, and lookup.
> nfsd4_sequence looks up the session id and if the session is found,
> it calls nfsd4_get_session (still under the sessionid_lock).
> nfsd4_destroy_session calls nfsd4_put_session after unhashing
> it, so when the session's kref reaches zero it's going to get freed.
>
> Signed-off-by: Benny Halevy <bhalevy@xxxxxxxxxxx>
> [we don't use a prime for sessionid hash table size]
> [use sessionid_lock spin lock]
> Signed-off-by: Benny Halevy <bhalevy@xxxxxxxxxxx>
> ---
> fs/nfsd/nfs4state.c | 57 +++++++++++++++++++++++++++++++++++++++++++-
> include/linux/nfsd/state.h | 7 +++++
> 2 files changed, 63 insertions(+), 1 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index df9d42e..ac4e8f2 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -383,11 +383,62 @@ static void release_openowner(struct nfs4_stateowner *sop)
> }
>
> #if defined(CONFIG_NFSD_V4_1)
> +static DEFINE_SPINLOCK(sessionid_lock);
> +#define SESSION_HASH_SIZE 512
> +static struct list_head sessionid_hashtbl[SESSION_HASH_SIZE];
> +
> +static inline int
> +hash_sessionid(struct nfs4_sessionid *sessionid)
> +{
> + struct nfsd4_sessionid *sid = (struct nfsd4_sessionid *)sessionid;
> +
> + return sid->sequence % SESSION_HASH_SIZE;
> +}
> +
> +static inline void
> +dump_sessionid(const char *fn, struct nfs4_sessionid *sessionid)
> +{
> + u32 *ptr = (u32 *)(&sessionid->data[0]);
> + dprintk("%s: %u:%u:%u:%u\n", fn, ptr[0], ptr[1], ptr[2], ptr[3]);
> +}
> +
> +/* caller must hold sessionid_lock */
> +static struct nfsd4_session *
> +find_in_sessionid_hashtbl(struct nfs4_sessionid *sessionid)
> +{
> + struct nfsd4_session *elem;
> + int idx;
> +
> + dump_sessionid(__func__, sessionid);
> + idx = hash_sessionid(sessionid);
> + dprintk("%s: idx is %d\n", __func__, idx);
> + /* Search in the appropriate list */
> + list_for_each_entry(elem, &sessionid_hashtbl[idx], se_hash) {
> + dump_sessionid("list traversal", &elem->se_sessionid);
> + if (!memcmp(elem->se_sessionid.data, sessionid->data,
> + NFS4_MAX_SESSIONID_LEN)) {
> + return elem;
> + }
> + }
> +
> + dprintk("%s: session not found\n", __func__);
Massive dprintk overkill in this function.
> + return NULL;
> +}
> +
> +/* caller must hold sessionid_lock */
> static void
> -release_session(struct nfsd4_session *ses)
> +unhash_session(struct nfsd4_session *ses)
> {
> list_del(&ses->se_hash);
> list_del(&ses->se_perclnt);
> +}
> +
> +static void
> +release_session(struct nfsd4_session *ses)
> +{
> + spin_lock(&sessionid_lock);
> + unhash_session(ses);
> + spin_unlock(&sessionid_lock);
> nfsd4_put_session(ses);
> }
It's not obvious from the names what the difference between
release_session() and nfsd4_put_session() is.
How about just renaming release_session() to unhash_session(), and
dumping hash_session? The two list_del()'s don't need their own
function.
--b.
>
> @@ -3213,6 +3264,10 @@ nfs4_state_init(void)
> INIT_LIST_HEAD(&unconf_str_hashtbl[i]);
> INIT_LIST_HEAD(&unconf_id_hashtbl[i]);
> }
> +#if defined(CONFIG_NFSD_V4_1)
> + for (i = 0; i < SESSION_HASH_SIZE; i++)
> + INIT_LIST_HEAD(&sessionid_hashtbl[i]);
> +#endif /* CONFIG_NFSD_V4_1 */
> for (i = 0; i < FILE_HASH_SIZE; i++) {
> INIT_LIST_HEAD(&file_hashtbl[i]);
> }
> diff --git a/include/linux/nfsd/state.h b/include/linux/nfsd/state.h
> index 29624b4..7592d7b 100644
> --- a/include/linux/nfsd/state.h
> +++ b/include/linux/nfsd/state.h
> @@ -133,6 +133,13 @@ nfsd4_get_session(struct nfsd4_session *ses)
> kref_get(&ses->se_ref);
> }
>
> +/* formatted contents of nfs4_sessionid */
> +struct nfsd4_sessionid {
> + clientid_t clientid;
> + u32 sequence;
> + u32 reserved;
> +};
> +
> #define HEXDIR_LEN 33 /* hex version of 16 byte md5 of cl_name plus '\0' */
>
> /*
> --
> 1.6.2.1
>
--
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
