On Feb 14, 2012, at 5:23 PM, Myklebust, Trond wrote:
> On Tue, 2012-02-14 at 16:23 -0500, Weston Andros Adamson wrote:
>> The caller of nfs4_find_slot() expects NFS4_MAX_SLOT_TABLE when no slot is
>> available (also mentioned in the comment header for nfs4_find_slot()).
>>
>> Signed-off-by: Weston Andros Adamson <dros@xxxxxxxxxx>
>> ---
>> This fixes an OOPS that is easy to reproduce:
>>
>> dd if=/dev/zero of=./zerofile bs=10024 count=1
>>
>> nfs4_find_slot() returns 2^32-1 which != NFS4_MAX_SLOT_TABLE, so it's used
>> as a valid slotid.
>>
>> Note that NFS4_NO_SLOT is still used for nfs4_slot_table::highest_used_slotid
>
> We need to fix the broken caller, not nfs4_find_slot()... I seem to have
> missed a couple of instances in the 'Convert slotid' patch…
>
> How's this?
Looks good - one minor issue below.
-dros
>
> 8<-----------------------------------------------------------------------------
> From 6e885e370726c4d72a24acfb06cbd6673e4b791f Mon Sep 17 00:00:00 2001
> From: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
> Date: Tue, 14 Feb 2012 17:11:57 -0500
> Subject: [PATCH] fixup! NFSv4.1: Convert slotid from u8 to u32
>
> Signed-off-by: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
> ---
> fs/nfs/nfs4proc.c | 21 ++++++++++-----------
> 1 files changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index da985c3..0b33165 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -362,16 +362,14 @@ static void renew_lease(const struct nfs_server *server, unsigned long timestamp
> * When updating highest_used_slotid there may be "holes" in the bitmap
> * so we need to scan down from highest_used_slotid to 0 looking for the now
> * highest slotid in use.
> - * If none found, highest_used_slotid is set to -1.
> + * If none found, highest_used_slotid is set to NFS4_NO_SLOT.
> *
> * Must be called while holding tbl->slot_tbl_lock
> */
> static void
> -nfs4_free_slot(struct nfs4_slot_table *tbl, u8 free_slotid)
> +nfs4_free_slot(struct nfs4_slot_table *tbl, u32 slotid)
> {
> - int slotid = free_slotid;
> -
> - BUG_ON(slotid < 0 || slotid >= NFS4_MAX_SLOT_TABLE);
> + BUG_ON(slotid >= NFS4_MAX_SLOT_TABLE);
> /* clear used bit in bitmap */
> __clear_bit(slotid, tbl->used_slots);
>
> @@ -381,10 +379,10 @@ nfs4_free_slot(struct nfs4_slot_table *tbl, u8 free_slotid)
> if (slotid < tbl->max_slots)
> tbl->highest_used_slotid = slotid;
> else
> - tbl->highest_used_slotid = -1;
> + tbl->highest_used_slotid = NFS4_NO_SLOT;
> }
> - dprintk("%s: free_slotid %u highest_used_slotid %d\n", __func__,
> - free_slotid, tbl->highest_used_slotid);
> + dprintk("%s: slotid %u highest_used_slotid %d\n", __func__,
> + slotid, tbl->highest_used_slotid);
%d -> %u, also %d -> %u in dprintk before return in nfs4_find_slot().
> }
>
> bool nfs4_set_task_privileged(struct rpc_task *task, void *dummy)
> @@ -512,7 +510,7 @@ static int nfs4_sequence_done(struct rpc_task *task,
> * nfs4_find_slot looks for an unset bit in the used_slots bitmap.
> * If found, we mark the slot as used, update the highest_used_slotid,
> * and respectively set up the sequence operation args.
> - * The slot number is returned if found, or NFS4_MAX_SLOT_TABLE otherwise.
> + * The slot number is returned if found, or NFS4_NO_SLOT otherwise.
> *
> * Note: must be called with under the slot_tbl_lock.
> */
> @@ -529,7 +527,8 @@ nfs4_find_slot(struct nfs4_slot_table *tbl)
> if (slotid >= tbl->max_slots)
> goto out;
> __set_bit(slotid, tbl->used_slots);
> - if (slotid > tbl->highest_used_slotid)
> + if (slotid > tbl->highest_used_slotid ||
> + tbl->highest_used_slotid == NFS4_NO_SLOT)
> tbl->highest_used_slotid = slotid;
> ret_id = slotid;
> out:
> @@ -584,7 +583,7 @@ int nfs41_setup_sequence(struct nfs4_session *session,
> }
>
> slotid = nfs4_find_slot(tbl);
> - if (slotid == NFS4_MAX_SLOT_TABLE) {
> + if (slotid == NFS4_NO_SLOT) {
> rpc_sleep_on(&tbl->slot_tbl_waitq, task, NULL);
> spin_unlock(&tbl->slot_tbl_lock);
> dprintk("<-- %s: no free slots\n", __func__);
> --
> 1.7.7.6
>
>
> --
> Trond Myklebust
> Linux NFS client maintainer
>
> NetApp
> Trond.Myklebust@xxxxxxxxxx
> www.netapp.com
>
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
