On Dec 11, 2011, at 8:14 AM, Benny Halevy wrote:
> Hi Andy,
>
> I have some minor-ish comments inline below, but the whole
> hlist concept looks to me like it might incur pretty high overhead
> in cpu and memory caching.
I'd like to measure the difference. What is the recommended way?
>
> Have you considered simply reallocating the slot table on grow
> and maybe on shrink below a certain threshold?
Because the slot pointer is placed in the response, we would have to drain the slot table to grow/shrink in this manner.
-->Andy
>
> On 2011-11-09 20:58, andros@xxxxxxxxxx wrote:
>> From: Andy Adamson <andros@xxxxxxxxxx>
>>
>> The session fore/backchannel slot tables implementation is changed from a
>> static array to an hlist hashed on slotid. Slotid's are values 0..N where N
>> is initially negotiated between the client and server via the CREATE_SESSION
>> call, and can be dynamically changed by the server during an active session.
>>
>> When there are no allocated slots (mount), an initial number of session slots
>> is allocated equal to the inital number of dynamic rpc_slots
>> created upon transport creation (xprt->min_reqs). Slots are then dynamically
>> allocated as needed upto the CREATE_SESSION negotiated max_reqs value.
>>
>> For session reset the session is drained (inactive). The (old) session has
>> allocated slot tables which we will reuse. nfs4_reduce_slots_locked is called
>> and only reduces the allocated_slots if the new max_reqs negotiated by the
>> reset CREATE_SESSION is less than the number of allocated_slots. The resultant
>> slot table is then re-initialized.
>>
>> CB_SLOT_RECALL is changed only in that it now shares code with session
>> reset - it also calls nfs4_reduce_slots_locked.
>>
>> The backchannel uses the new dynamic slot allocator, even though we only
>> currently support a single back channel slot.
>>
>> Signed-off-by: Andy Adamson <andros@xxxxxxxxxx>
>> ---
>> fs/nfs/callback_proc.c | 25 +++-
>> fs/nfs/internal.h | 2 +
>> fs/nfs/nfs4_fs.h | 1 +
>> fs/nfs/nfs4proc.c | 297 ++++++++++++++++++++++++++++++++++-----------
>> fs/nfs/nfs4state.c | 12 +--
>> fs/nfs/nfs4xdr.c | 20 ++--
>> include/linux/nfs_fs_sb.h | 12 +-
>> include/linux/nfs_xdr.h | 4 +-
>> 8 files changed, 266 insertions(+), 107 deletions(-)
>>
>> diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
>> index 54cea8a..77cc96c 100644
>> --- a/fs/nfs/callback_proc.c
>> +++ b/fs/nfs/callback_proc.c
>> @@ -342,7 +342,7 @@ validate_seqid(struct nfs4_slot_table *tbl, struct cb_sequenceargs * args)
>> if (args->csa_slotid >= NFS41_BC_MAX_CALLBACKS)
>> return htonl(NFS4ERR_BADSLOT);
>>
>> - slot = tbl->slots + args->csa_slotid;
>> + slot = nfs4_lookup_slot_locked(tbl, args->csa_slotid);
>> dprintk("%s slot table seqid: %d\n", __func__, slot->seq_nr);
>>
>> /* Normal */
>> @@ -377,6 +377,22 @@ out_ok:
>> return htonl(NFS4_OK);
>> }
>>
>> +static bool test_slot_referring(struct nfs4_slot_table *tbl,
>> + struct referring_call *ref)
>> +{
>> + struct nfs4_slot *slot;
>> + bool status = false;
>> +
>> + spin_lock(&tbl->slot_tbl_lock);
>> + if (test_bit(ref->rc_slotid, tbl->used_slots)) {
>> + slot = nfs4_lookup_slot_locked(tbl, ref->rc_slotid);
>> + if (slot->seq_nr == ref->rc_sequenceid)
>> + status = true;
>> + }
>> + spin_unlock(&tbl->slot_tbl_lock);
>> + return status;
>> +}
>> +
>> /*
>> * For each referring call triple, check the session's slot table for
>> * a match. If the slot is in use and the sequence numbers match, the
>> @@ -417,12 +433,7 @@ static bool referring_call_exists(struct nfs_client *clp,
>> ((u32 *)&rclist->rcl_sessionid.data)[2],
>> ((u32 *)&rclist->rcl_sessionid.data)[3],
>> ref->rc_sequenceid, ref->rc_slotid);
>> -
>> - spin_lock(&tbl->slot_tbl_lock);
>> - status = (test_bit(ref->rc_slotid, tbl->used_slots) &&
>> - tbl->slots[ref->rc_slotid].seq_nr ==
>> - ref->rc_sequenceid);
>> - spin_unlock(&tbl->slot_tbl_lock);
>> + status = test_slot_referring(tbl, ref);
>> if (status)
>> goto out;
>> }
>> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
>> index c1a1bd8..4451abf 100644
>> --- a/fs/nfs/internal.h
>> +++ b/fs/nfs/internal.h
>> @@ -354,6 +354,8 @@ extern int _nfs4_call_sync_session(struct rpc_clnt *clnt,
>> struct nfs4_sequence_args *args,
>> struct nfs4_sequence_res *res,
>> int cache_reply);
>> +struct nfs4_slot *nfs4_lookup_slot_locked(struct nfs4_slot_table *tbl,
>> + u8 slotid);
>>
>> /*
>> * Determine the device name as a string
>> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
>> index 693ae22..ff87169 100644
>> --- a/fs/nfs/nfs4_fs.h
>> +++ b/fs/nfs/nfs4_fs.h
>> @@ -242,6 +242,7 @@ extern struct nfs4_session *nfs4_alloc_session(struct nfs_client *clp);
>> extern int nfs4_proc_create_session(struct nfs_client *);
>> extern int nfs4_proc_destroy_session(struct nfs4_session *);
>> extern int nfs4_init_session(struct nfs_server *server);
>> +extern void nfs4_reduce_slots_locked(struct nfs4_slot_table *tbl);
>> extern int nfs4_proc_get_lease_time(struct nfs_client *clp,
>> struct nfs_fsinfo *fsinfo);
>> extern int nfs4_proc_layoutcommit(struct nfs4_layoutcommit_data *data,
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index 2638291..e3a7663 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -346,6 +346,162 @@ static void renew_lease(const struct nfs_server *server, unsigned long timestamp
>> #if defined(CONFIG_NFS_V4_1)
>>
>> /*
>> + * Session dynamic slot table helper functions
>> + */
>> +
>> +static inline u32 slot_tbl_hash(const u8 slotid)
>
> nit: "const" not really needed here...
>
>> +{
>> + return (u32)slotid % SLOT_TABLE_SIZE;
>> +}
>> +
>> +static void nfs4_init_slot(struct nfs4_slot *new, u8 slotid, int ivalue)
>> +{
>> + INIT_HLIST_NODE(&new->slot_node);
>> + new->slotid = slotid;
>> + new->seq_nr = ivalue;
>> +}
>> +
>> +static void nfs4_insert_slot_locked(struct nfs4_slot_table *tbl,
>> + struct nfs4_slot *new)
>> +{
>> + u32 hash = slot_tbl_hash(new->slotid);
>> +
>> + hlist_add_head(&new->slot_node, &tbl->slots[hash]);
>> + tbl->allocated_slots++;
>> +}
>> +
>> +/*
>> + * Allocate a slot for the next slotid (tbl->allocated_slotid)
>> + */
>> +static int nfs4_alloc_add_slot(struct nfs4_slot_table *tbl,
>> + int ivalue, int slotid,
>> + gfp_t gfp_flags)
>> +{
>> + struct nfs4_slot *new;
>> + struct rpc_task *task;
>> +
>> + dprintk("--> %s slotid=%u tbl->allocated_slots=%d\n", __func__,
>> + slotid, tbl->allocated_slots);
>> + new = kzalloc(sizeof(struct nfs4_slot), gfp_flags);
>> + if (!new)
>> + return -ENOMEM;
>> + /* tbl->allocated_slots holds the next unallocated slotid */
>> + nfs4_init_slot(new, slotid, ivalue);
>> + spin_lock(&tbl->slot_tbl_lock);
>> + if (tbl->allocated_slots != slotid) {
>
> This condition doesn't seem to be reliable.
> What if the slot table shrunk?
>
>> + spin_unlock(&tbl->slot_tbl_lock);
>> + kfree(new);
>> + dprintk("%s slotid %u already allocated\n", __func__,
>> + slotid);
>> + return 0;
>> + }
>> + nfs4_insert_slot_locked(tbl, new);
>> +
>> + /* This only applies to dynamic allocation */
>> + task = rpc_wake_up_next(&tbl->slot_tbl_waitq);
>> + if (task)
>> + dprintk("%s woke up task %5u\n", __func__, task->tk_pid);
>> + spin_unlock(&tbl->slot_tbl_lock);
>> + return 0;
>> +}
>> +
>> +/*
>> + * Allocate the minimal number of slots required by the transport.
>> + * Called at session creation.
>> + */
>> +static int nfs4_prealloc_slots(struct nfs4_slot_table *tbl, int num, int ivalue)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < num; i++) {
>> + if (nfs4_alloc_add_slot(tbl, ivalue, tbl->allocated_slots,
>> + GFP_NOFS) != 0)
>> + return -ENOMEM;
>
> Returning nfs4_alloc_add_slot's status would be more appropriate.
> How about also freeing 0 to i-1 in the error case?
>
>> + }
>> + return 0;
>> +}
>> +
>> +/*
>> + * Allocate the next slotid. If allocation fails, next request will retry.
>> + */
>> +static void nfs4_dynamic_alloc_slot(struct nfs4_slot_table *tbl, u8 slotid)
>> +{
>> + if (slotid != tbl->allocated_slots) {
>> + dprintk("%s slotid %u already allocated\n", __func__, slotid);
>> + return;
>> + }
>
> This is checked again under the lock in nfs4_alloc_add_slot, no?
> so if it is not supposed to be a common case (which I don't believe
> it is) I'd just forget about this optimization and call the latter
> directly.
>
>> + nfs4_alloc_add_slot(tbl, 1, slotid, GFP_NOWAIT);
>> +}
>> +
>> +/*
>> + * Return the slot associated with the slotid returned by nfs4_find_slot.
>> + * If the slot is not found, it has not been allocated.
>> + */
>> +struct nfs4_slot *
>> +nfs4_lookup_slot_locked(struct nfs4_slot_table *tbl, u8 slotid)
>> +{
>> + struct nfs4_slot *sp;
>> + struct hlist_node *n;
>> + u32 hash = slot_tbl_hash(slotid);
>> +
>> + hlist_for_each_entry(sp, n, &tbl->slots[hash], slot_node) {
>> + if (sp->slotid == slotid)
>> + return sp;
>> + }
>> + return NULL;
>> +}
>> +
>> +static void nfs4_remove_slot_locked(struct nfs4_slot_table *tbl,
>> + struct nfs4_slot *slot)
>> +{
>> + dprintk("--> %s slotid %d\n", __func__, slot->slotid);
>> + hlist_del_init(&slot->slot_node);
>> + tbl->allocated_slots--;
>> + kfree(slot);
>> +}
>> +
>> +static void nfs4_free_all_slots(struct nfs4_slot_table *tbl)
>> +{
>> + struct nfs4_slot *slotp;
>> + int i;
>> +
>> + spin_lock(&tbl->slot_tbl_lock);
>> + for (i = 0; i < SLOT_TABLE_SIZE; i++) {
>> + while (!hlist_empty(&tbl->slots[i])) {
>> + slotp = hlist_entry(tbl->slots[i].first,
>> + struct nfs4_slot, slot_node);
>> + nfs4_remove_slot_locked(tbl, slotp);
>> + }
>> + /* re-initialize hlist_head */
>> + tbl->slots[i].first = NULL;
>> + }
>> + spin_unlock(&tbl->slot_tbl_lock);
>> +}
>> +
>> +/*
>> + * Remove num_dec number of contiguous slotids starting from highest
>> + * allocated slotid
>> + *
>> + * Note: set the new tbl->max_slots prior to calling.
>> + */
>> +void nfs4_reduce_slots_locked(struct nfs4_slot_table *tbl)
>> +{
>> + struct nfs4_slot *removeme;
>> + int i, rm_slotid = tbl->allocated_slots - 1;
>> + int num_dec = tbl->allocated_slots - tbl->max_slots;
>> +
>> + /* Reset of the slot table can make num_dec <= 0 */
>> + if (num_dec <= 0)
>> + return;
>> + for (i = 0; i < num_dec; i++) {
>> + removeme = nfs4_lookup_slot_locked(tbl, rm_slotid);
>> + BUG_ON(removeme == NULL);
>> + nfs4_remove_slot_locked(tbl, removeme);
>> + rm_slotid--;
>> + }
>> +}
>> +
>> +/*
>> * nfs4_free_slot - free a slot and efficiently update slot table.
>> *
>> * freeing a slot is trivially done by clearing its respective bit
>> @@ -390,8 +546,11 @@ static void nfs4_check_drain_fc_complete(struct nfs4_session *ses)
>>
>> if (!test_bit(NFS4_SESSION_DRAINING, &ses->session_state)) {
>> task = rpc_wake_up_next(&ses->fc_slot_table.slot_tbl_waitq);
>> - if (task)
>> + if (task) {
>> rpc_task_set_priority(task, RPC_PRIORITY_PRIVILEGED);
>> + dprintk("%s woke up task pid %5u\n", __func__,
>> + task->tk_pid);
>> + }
>> return;
>> }
>>
>> @@ -427,7 +586,7 @@ static void nfs41_sequence_free_slot(struct nfs4_sequence_res *res)
>> }
>>
>> spin_lock(&tbl->slot_tbl_lock);
>> - nfs4_free_slot(tbl, res->sr_slot - tbl->slots);
>> + nfs4_free_slot(tbl, res->sr_slot->slotid);
>> nfs4_check_drain_fc_complete(res->sr_session);
>> spin_unlock(&tbl->slot_tbl_lock);
>> res->sr_slot = NULL;
>> @@ -468,10 +627,8 @@ static int nfs41_sequence_done(struct rpc_task *task, struct nfs4_sequence_res *
>> * returned NFS4ERR_DELAY as per Section 2.10.6.2
>> * of RFC5661.
>> */
>> - dprintk("%s: slot=%td seq=%d: Operation in progress\n",
>> - __func__,
>> - res->sr_slot - res->sr_session->fc_slot_table.slots,
>> - res->sr_slot->seq_nr);
>> + dprintk("%s: slotid=%u seq=%d: Operation in progress\n",
>> + __func__, res->sr_slot->slotid, res->sr_slot->seq_nr);
>> goto out_retry;
>> default:
>> /* Just update the slot sequence no. */
>> @@ -479,7 +636,8 @@ static int nfs41_sequence_done(struct rpc_task *task, struct nfs4_sequence_res *
>> }
>> out:
>> /* The session may be reset by one of the error handlers. */
>> - dprintk("%s: Error %d free the slot \n", __func__, res->sr_status);
>> + dprintk("%s: Error %d free the slot for task pid %5u\n", __func__,
>> + res->sr_status, task->tk_pid);
>> nfs41_sequence_free_slot(res);
>> return 1;
>> out_retry:
>> @@ -540,7 +698,7 @@ int nfs41_setup_sequence(struct nfs4_session *session,
>> struct nfs4_slot_table *tbl;
>> u8 slotid;
>>
>> - dprintk("--> %s\n", __func__);
>> + dprintk("--> %s task pid %5u\n", __func__, task->tk_pid);
>> /* slot already allocated? */
>> if (res->sr_slot != NULL)
>> return 0;
>> @@ -575,12 +733,24 @@ int nfs41_setup_sequence(struct nfs4_session *session,
>> dprintk("<-- %s: no free slots\n", __func__);
>> return -EAGAIN;
>> }
>> + slot = nfs4_lookup_slot_locked(tbl, slotid);
>> + if (slot == NULL) {
>> + /* keep FIFO order */
>> + rpc_task_set_priority(task, RPC_PRIORITY_PRIVILEGED);
>> + rpc_sleep_on(&tbl->slot_tbl_waitq, task, NULL);
>> + nfs4_free_slot(tbl, slotid);
>
> why free the slot here?
>
> Benny
>
>> + spin_unlock(&tbl->slot_tbl_lock);
>> + dprintk("%s task pid %5u sleeping for dynamic slot %d\n",
>> + __func__, task->tk_pid, slotid);
>> + /* rpc_wakeup_next called upon success by slot allocator */
>> + nfs4_dynamic_alloc_slot(tbl, slotid);
>> + return -EAGAIN;
>> + }
>> spin_unlock(&tbl->slot_tbl_lock);
>>
>> rpc_task_set_priority(task, RPC_PRIORITY_NORMAL);
>> - slot = tbl->slots + slotid;
>> args->sa_session = session;
>> - args->sa_slotid = slotid;
>> + args->sa_slot = slot;
>> args->sa_cache_this = cache_reply;
>>
>> dprintk("<-- %s slotid=%d seqid=%d\n", __func__, slotid, slot->seq_nr);
>> @@ -613,9 +783,9 @@ int nfs4_setup_sequence(const struct nfs_server *server,
>> goto out;
>> }
>>
>> - dprintk("--> %s clp %p session %p sr_slot %td\n",
>> + dprintk("--> %s clp %p session %p sr_slotid %d\n",
>> __func__, session->clp, session, res->sr_slot ?
>> - res->sr_slot - session->fc_slot_table.slots : -1);
>> + res->sr_slot->slotid : -1);
>>
>> ret = nfs41_setup_sequence(session, args, res, cache_reply,
>> task);
>> @@ -4976,84 +5146,64 @@ int nfs4_proc_get_lease_time(struct nfs_client *clp, struct nfs_fsinfo *fsinfo)
>> }
>>
>> /*
>> - * Reset a slot table
>> + * Reset a slot table. Session has been drained and reset with new slot table
>> + * parameters from create_session.
>> + * Keep all allocated slots that fit into new slot table max_reqs parameter.
>> */
>> -static int nfs4_reset_slot_table(struct nfs4_slot_table *tbl, u32 max_reqs,
>> +static void nfs4_reset_slot_table(struct nfs4_slot_table *tbl, u32 new_max_reqs,
>> int ivalue)
>> {
>> - struct nfs4_slot *new = NULL;
>> + struct nfs4_slot *sp;
>> + struct hlist_node *n;
>> int i;
>> - int ret = 0;
>> -
>> - dprintk("--> %s: max_reqs=%u, tbl->max_slots %d\n", __func__,
>> - max_reqs, tbl->max_slots);
>>
>> - /* Does the newly negotiated max_reqs match the existing slot table? */
>> - if (max_reqs != tbl->max_slots) {
>> - ret = -ENOMEM;
>> - new = kmalloc(max_reqs * sizeof(struct nfs4_slot),
>> - GFP_NOFS);
>> - if (!new)
>> - goto out;
>> - ret = 0;
>> - kfree(tbl->slots);
>> - }
>> spin_lock(&tbl->slot_tbl_lock);
>> - if (new) {
>> - tbl->slots = new;
>> - tbl->max_slots = max_reqs;
>> + tbl->max_slots = new_max_reqs;
>> + nfs4_reduce_slots_locked(tbl);
>> + tbl->highest_used_slotid = -1;
>> +
>> + for (i = 0; i < SLOT_TABLE_SIZE; i++) {
>> + hlist_for_each_entry(sp, n, &tbl->slots[i], slot_node)
>> + sp->seq_nr = ivalue;
>> }
>> - for (i = 0; i < tbl->max_slots; ++i)
>> - tbl->slots[i].seq_nr = ivalue;
>> spin_unlock(&tbl->slot_tbl_lock);
>> - dprintk("%s: tbl=%p slots=%p max_slots=%d\n", __func__,
>> - tbl, tbl->slots, tbl->max_slots);
>> -out:
>> - dprintk("<-- %s: return %d\n", __func__, ret);
>> - return ret;
>> + dprintk("%s: tbl->max_slots=%d tbl->allocated_slots=%d\n",
>> + __func__, tbl->max_slots, tbl->allocated_slots);
>> + return;
>> }
>>
>> /* Destroy the slot table */
>> static void nfs4_destroy_slot_tables(struct nfs4_session *session)
>> {
>> - if (session->fc_slot_table.slots != NULL) {
>> - kfree(session->fc_slot_table.slots);
>> - session->fc_slot_table.slots = NULL;
>> - }
>> - if (session->bc_slot_table.slots != NULL) {
>> - kfree(session->bc_slot_table.slots);
>> - session->bc_slot_table.slots = NULL;
>> - }
>> - return;
>> + dprintk("--> %s\n", __func__);
>> +
>> + nfs4_free_all_slots(&session->fc_slot_table);
>> + nfs4_free_all_slots(&session->bc_slot_table);
>> }
>>
>> /*
>> * Initialize slot table
>> */
>> static int nfs4_init_slot_table(struct nfs4_slot_table *tbl,
>> - int max_slots, int ivalue)
>> + int max_slots, int num_prealloc, int ivalue)
>> {
>> - struct nfs4_slot *slot;
>> - int ret = -ENOMEM;
>> + int ret;
>>
>> BUG_ON(max_slots > NFS4_MAX_SLOT_TABLE);
>> -
>> - dprintk("--> %s: max_reqs=%u\n", __func__, max_slots);
>> -
>> - slot = kcalloc(max_slots, sizeof(struct nfs4_slot), GFP_NOFS);
>> - if (!slot)
>> + ret = nfs4_prealloc_slots(tbl, num_prealloc, ivalue);
>> + if (ret) {
>> + nfs4_free_all_slots(tbl);
>> goto out;
>> + }
>> ret = 0;
>>
>> spin_lock(&tbl->slot_tbl_lock);
>> tbl->max_slots = max_slots;
>> - tbl->slots = slot;
>> tbl->highest_used_slotid = -1; /* no slot is currently used */
>> spin_unlock(&tbl->slot_tbl_lock);
>> - dprintk("%s: tbl=%p slots=%p max_slots=%d\n", __func__,
>> - tbl, tbl->slots, tbl->max_slots);
>> + dprintk("%s: tbl=%p max_slots=%d, tbl->allocated_slots=%d\n", __func__,
>> + tbl, tbl->max_slots, tbl->allocated_slots);
>> out:
>> - dprintk("<-- %s: return %d\n", __func__, ret);
>> return ret;
>> }
>>
>> @@ -5063,30 +5213,34 @@ out:
>> static int nfs4_setup_session_slot_tables(struct nfs4_session *ses)
>> {
>> struct nfs4_slot_table *tbl;
>> - int status;
>> + int status = 0;
>>
>> dprintk("--> %s\n", __func__);
>> /* Fore channel */
>> tbl = &ses->fc_slot_table;
>> - if (tbl->slots == NULL) {
>> - status = nfs4_init_slot_table(tbl, ses->fc_attrs.max_reqs, 1);
>> + if (tbl->allocated_slots == 0) {
>> + status = nfs4_init_slot_table(tbl, ses->fc_attrs.max_reqs,
>> + ses->clp->cl_rpcclient->cl_xprt->min_reqs, 1);
>> if (status) /* -ENOMEM */
>> return status;
>> } else {
>> - status = nfs4_reset_slot_table(tbl, ses->fc_attrs.max_reqs, 1);
>> - if (status)
>> - return status;
>> + nfs4_reset_slot_table(tbl, ses->fc_attrs.max_reqs, 1);
>> }
>> +
>> /* Back channel */
>> tbl = &ses->bc_slot_table;
>> - if (tbl->slots == NULL) {
>> - status = nfs4_init_slot_table(tbl, ses->bc_attrs.max_reqs, 0);
>> + if (tbl->allocated_slots == 0) {
>> + status = nfs4_init_slot_table(tbl, ses->bc_attrs.max_reqs,
>> + /* Backchannel max_reqs == 1 */
>> + ses->bc_attrs.max_reqs, 0);
>> if (status)
>> /* Fore and back channel share a connection so get
>> * both slot tables or neither */
>> nfs4_destroy_slot_tables(ses);
>> - } else
>> - status = nfs4_reset_slot_table(tbl, ses->bc_attrs.max_reqs, 0);
>> + } else {
>> + nfs4_reset_slot_table(tbl, ses->bc_attrs.max_reqs, 0);
>> + }
>> +
>> return status;
>> }
>>
>> @@ -5276,7 +5430,6 @@ int nfs4_proc_create_session(struct nfs_client *clp)
>>
>> /* Init or reset the session slot tables */
>> status = nfs4_setup_session_slot_tables(session);
>> - dprintk("slot table setup returned %d\n", status);
>> if (status)
>> goto out;
>>
>> @@ -5284,7 +5437,7 @@ int nfs4_proc_create_session(struct nfs_client *clp)
>> dprintk("%s client>seqid %d sessionid %u:%u:%u:%u\n", __func__,
>> clp->cl_seqid, ptr[0], ptr[1], ptr[2], ptr[3]);
>> out:
>> - dprintk("<-- %s\n", __func__);
>> + dprintk("<-- %s status %d\n", __func__, status);
>> return status;
>> }
>>
>> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
>> index 39914be..8260865 100644
>> --- a/fs/nfs/nfs4state.c
>> +++ b/fs/nfs/nfs4state.c
>> @@ -1573,26 +1573,16 @@ static int nfs4_recall_slot(struct nfs_client *clp)
>> {
>> struct nfs4_slot_table *fc_tbl = &clp->cl_session->fc_slot_table;
>> struct nfs4_channel_attrs *fc_attrs = &clp->cl_session->fc_attrs;
>> - struct nfs4_slot *new, *old;
>> - int i;
>>
>> nfs4_begin_drain_session(clp);
>> - new = kmalloc(fc_tbl->target_max_slots * sizeof(struct nfs4_slot),
>> - GFP_NOFS);
>> - if (!new)
>> - return -ENOMEM;
>>
>> spin_lock(&fc_tbl->slot_tbl_lock);
>> - for (i = 0; i < fc_tbl->target_max_slots; i++)
>> - new[i].seq_nr = fc_tbl->slots[i].seq_nr;
>> - old = fc_tbl->slots;
>> - fc_tbl->slots = new;
>> fc_tbl->max_slots = fc_tbl->target_max_slots;
>> + nfs4_reduce_slots_locked(fc_tbl);
>> fc_tbl->target_max_slots = 0;
>> fc_attrs->max_reqs = fc_tbl->max_slots;
>> spin_unlock(&fc_tbl->slot_tbl_lock);
>>
>> - kfree(old);
>> nfs4_end_drain_session(clp);
>> return 0;
>> }
>> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
>> index e6161b2..c42ec29 100644
>> --- a/fs/nfs/nfs4xdr.c
>> +++ b/fs/nfs/nfs4xdr.c
>> @@ -1872,7 +1872,6 @@ static void encode_sequence(struct xdr_stream *xdr,
>> #if defined(CONFIG_NFS_V4_1)
>> struct nfs4_session *session = args->sa_session;
>> struct nfs4_slot_table *tp;
>> - struct nfs4_slot *slot;
>> __be32 *p;
>>
>> if (!session)
>> @@ -1880,8 +1879,7 @@ static void encode_sequence(struct xdr_stream *xdr,
>>
>> tp = &session->fc_slot_table;
>>
>> - WARN_ON(args->sa_slotid == NFS4_MAX_SLOT_TABLE);
>> - slot = tp->slots + args->sa_slotid;
>> + WARN_ON(args->sa_slot->slotid == NFS4_MAX_SLOT_TABLE);
>>
>> p = reserve_space(xdr, 4 + NFS4_MAX_SESSIONID_LEN + 16);
>> *p++ = cpu_to_be32(OP_SEQUENCE);
>> @@ -1896,11 +1894,11 @@ static void encode_sequence(struct xdr_stream *xdr,
>> ((u32 *)session->sess_id.data)[1],
>> ((u32 *)session->sess_id.data)[2],
>> ((u32 *)session->sess_id.data)[3],
>> - slot->seq_nr, args->sa_slotid,
>> + args->sa_slot->seq_nr, args->sa_slot->slotid,
>> tp->highest_used_slotid, args->sa_cache_this);
>> p = xdr_encode_opaque_fixed(p, session->sess_id.data, NFS4_MAX_SESSIONID_LEN);
>> - *p++ = cpu_to_be32(slot->seq_nr);
>> - *p++ = cpu_to_be32(args->sa_slotid);
>> + *p++ = cpu_to_be32(args->sa_slot->seq_nr);
>> + *p++ = cpu_to_be32(args->sa_slot->slotid);
>> *p++ = cpu_to_be32(tp->highest_used_slotid);
>> *p = cpu_to_be32(args->sa_cache_this);
>> hdr->nops++;
>> @@ -5361,13 +5359,17 @@ static int decode_sequence(struct xdr_stream *xdr,
>> /* seqid */
>> dummy = be32_to_cpup(p++);
>> if (dummy != res->sr_slot->seq_nr) {
>> - dprintk("%s Invalid sequence number\n", __func__);
>> + dprintk("%s Invalid seq num %d for [slotid:seq_nr] [%d:%d]\n",
>> + __func__, dummy, res->sr_slot->slotid,
>> + res->sr_slot->seq_nr);
>> goto out_err;
>> }
>> /* slot id */
>> dummy = be32_to_cpup(p++);
>> - if (dummy != res->sr_slot - res->sr_session->fc_slot_table.slots) {
>> - dprintk("%s Invalid slot id\n", __func__);
>> + if (dummy != res->sr_slot->slotid) {
>> + dprintk("%s Invalid slot id %d for [slotid:seq_nr] [%d:%d]\n",
>> + __func__, dummy, res->sr_slot->slotid,
>> + res->sr_slot->seq_nr);
>> goto out_err;
>> }
>> /* highest slot id - currently not processed */
>> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
>> index b5479df..90c6e2b 100644
>> --- a/include/linux/nfs_fs_sb.h
>> +++ b/include/linux/nfs_fs_sb.h
>> @@ -193,12 +193,15 @@ struct nfs_server {
>>
>> /* Sessions */
>> #define SLOT_TABLE_SZ (NFS4_MAX_SLOT_TABLE/(8*sizeof(long)))
>> +#define SLOT_TABLE_BITS 5
>> +#define SLOT_TABLE_SIZE (1 << SLOT_TABLE_BITS)
>> struct nfs4_slot_table {
>> - struct nfs4_slot *slots; /* seqid per slot */
>> + struct hlist_head slots[SLOT_TABLE_SIZE];
>> unsigned long used_slots[SLOT_TABLE_SZ]; /* used/unused bitmap */
>> spinlock_t slot_tbl_lock;
>> struct rpc_wait_queue slot_tbl_waitq; /* allocators may wait here */
>> - int max_slots; /* # slots in table */
>> + int allocated_slots; /* # slots in table */
>> + int max_slots; /* max # slots in table */
>> int highest_used_slotid; /* sent to server on each SEQ.
>> * op for dynamic resizing */
>> int target_max_slots; /* Set by CB_RECALL_SLOT as
>> @@ -206,11 +209,6 @@ struct nfs4_slot_table {
>> struct completion complete;
>> };
>>
>> -static inline int slot_idx(struct nfs4_slot_table *tbl, struct nfs4_slot *sp)
>> -{
>> - return sp - tbl->slots;
>> -}
>> -
>> /*
>> * Session related parameters
>> */
>> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
>> index c74595b..76b27a4 100644
>> --- a/include/linux/nfs_xdr.h
>> +++ b/include/linux/nfs_xdr.h
>> @@ -168,12 +168,14 @@ struct nfs4_channel_attrs {
>>
>> /* nfs41 sessions slot seqid */
>> struct nfs4_slot {
>> + struct hlist_node slot_node;
>> u32 seq_nr;
>> + u8 slotid;
>> };
>>
>> struct nfs4_sequence_args {
>> struct nfs4_session *sa_session;
>> - u8 sa_slotid;
>> + struct nfs4_slot *sa_slot;
>> u8 sa_cache_this;
>> };
>>
--
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
