On Wed, Jun 16, 2010 at 9:41 AM, Trond Myklebust <trond.myklebust@xxxxxxxxxx> wrote: > On Tue, 2010-06-15 at 18:31 -0400, andros@xxxxxxxxxx wrote: >> From: Andy Adamson <andros@xxxxxxxxxx> >> >> _nfs41_proc_sequence() kmallocs the calldata but does not set >> calldata->res.slotid to NFS4_MAX_SLOT_TABLE, so nfs41_sequence_prepare() calls >> nfs41_setup_sequence() with the res.sr_slotid set to a random value. >> If nfs41_setup_sequence() sees res.sr_slotid as not set to NFS4_MAX_SLOT_TABLE, >> it returns without actually setting up the sequence. > > Looks like a typo in my patch "NFSv41: Fix a memory leak in > nfs41_proc_async_sequence()". I'll squash your patch into that. Please > note that you will have to rebase if you try to pull the nfs-for-2.6.36 > branch again. > > Now.... With that said, do we really need to allow RPC calls to call > nfs41_setup_sequence with an initialised slotid? nfs41_setup_sequence needs to know when a slot has already been initialized which happens when the error handlers resend. > This initialisation is > a serious hassle, and is easy to forget. Agreed. I'll make it easy to remember by coding an initialization helper function. > When do we expect to end up hitting the res.sr_slotid != > NFS4_MAX_SLOT_TABLE condition in real life? What we don't expect in real life is for session->max_req == NFS4_MAX_SLOT_TABLE, unless it is not initialized. A valid slotid is any value between 0 and session->max_req e.g. slotid != NFS4_MAX_SLOT_TABLE, so we hit that condition every valid SEQUENCE op. -->Andy > > Cheers > Trond > > -- > 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
