On Wed, Feb 15, 2012 at 10:46 AM, Myklebust, Trond <Trond.Myklebust@xxxxxxxxxx> wrote: > On Wed, 2012-02-15 at 15:07 +0000, Adamson, Andy wrote: >> >> Please don't merge nfs4_reset_slot_tables and nfs4_init_slot_tables. It assumes a static array (realloc) which goes away with the dynamic slot table code. Instead, take a look at the patch set I sent on Feb 12 > > Why not? There is _no_ functional difference between what > nfs4_reset_slot_tables() and nfs4_init_slot_tables() need to do. > They both need to allocate new slots (conditionally in the case of > nfs4_reset_slot_tables, but the condition is compatible with the > nfs4_init_slot_tables case), and they both need to initialise those > slots to the value '1'. The reset case also needs to reduce the number of slots. In the dynamic slot case, the reset function will not require draining of the session. So reset will, in response to a change in the target_highest_slotid or a CB_RECALL_SLOT from the server, add or remove slots without draining the session. In this case, reset will only initialize any new slots sequence numbers to 1, not all slots. That said, I guess it can all be in one function. > > AFAICS There is no reason for keeping those as 2 separate functions, and > I don't see how the dynamic session patches change anything to that > conclusion. > >> [PATCH Version 7 3/3] NFSv4.1 avoid freeing slot when tasks are waiting >> [PATCH Version 1 2/3] NFSv4.1 prepare for dyamic session slots >> [PATCH Version 7 1/3] NFS4.1 clean up nfs4_alloc_session > > I need a fix for the 3.3 final... > > Those patches can be cleaned up and made ready for 3.4 (needs work - > they won't apply to the 'nfs-for-next' branch), I worked against the bugfix branch. I'll use the nfs-for-next branch. > but right now they're > not sufficiently tested nor are they sufficiently reviewed. Agreed. I want to test some more at connectathon. I did iozone tests to show that this dynamic slot implementation performs as well as the static array, but not enough testing / review for coding bugs. > For > instance, I'm not happy with the idea of adding a 'tk_private' field in > the struct rpc_task. OK. Do you have a suggestion? Thanks -->Andy > > > -- > Trond Myklebust > Linux NFS client maintainer > > NetApp > Trond.Myklebust@xxxxxxxxxx > www.netapp.com > -- 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
