On 2011-12-12 18:11, Adamson, Andy wrote:
>
> 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?
>
I think that the best way is to create a synthetic storm of SEQUENCE
COMPOUNDs on random slots that also expand or shrink the slot table
in a pre-defined probability. The metric would be ops / sec
at saturation.
>>
>> 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.
Maybe you are optimizing for the rare case rather than for the common case.
It might not be straight forward but I think we can leverage the exactly
once semantics and since slots cannot be reused while their respective
RPC is in progress, we keep the old table around after cloning it to a newly
allocated array and release the old one only after it is drained.
the trickier parts would be to keep a correct reference count
on the table (doesn't seem to be too hard to do under the lock)
and update the new table on completion rather than the old one
when it's marked as being resized.
Benny
>
> -->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
--
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
