On Fri, 15 Apr 2011 08:38:59 -0500 Steve French <smfrench@xxxxxxxxx> wrote: > On Fri, Apr 15, 2011 at 5:45 AM, Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote: > > 2011/4/8 Steve French <smfrench@xxxxxxxxx>: > >> Update on cifs vs. smb2 mids, and the smb2 sendrcv2. Â Jeff had > >> suggested more closely matching the cifs and smb2 mids, in particular > >> extending the 16 bit cifs mid (multiplex identifier for inflight > >> network requests) to the 64 bit size needed for smb2 (and thus masking > >> the mid when used for cifs) and having cifs ignore the various smb2 > >> unique fields in the mid (which makes the mid larger for cifs). > >> Since the smb2 code in cifs-2.6.git (put in February and early March) > >> has now been rereviewed, the next step in the smb2 merge is posting > >> and reviewing the transport routine for smb2 (smb2_sendrcv2 or reusing > >> cifs_sendrcv2) - the latter may make more sense if we go to a common > >> mid for cifs and smb2. Â At the fs summit, Jeff and Jeremy and I > >> talked about this, but Pavel and others may have opinions on this > >> topic. Â As soon as the cifs merge activity settles down for 2.6.39, I > >> plan to post sendrcv2 alternatives and then begin work with Pavel on > >> the superblock, file and inode routines and seeing whether for smb2 > >> they should be smb2 unique (as we originally expected since smb2 is > >> handle based, and simpler) and look more like they did in the smb2.ko > >> work that Pavel did last summer or should be more common with the cifs > >> routines. > >> > >> -- > >> Thanks, > >> > >> Steve > >> -- > >> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in > >> the body of a message to majordomo@xxxxxxxxxxxxxxx > >> More majordomo info at Âhttp://vger.kernel.org/majordomo-info.html > >> > > > > I suggest to make cifs and smb2 protocol mid structures use common > > structure that has equals fields for both and then expand this > > structure for protocol-dependent things. > > > > It can look like this: > > > > /* one of these for every pending CIFS request to the server */ > > struct mid_q_entry { > > Â Â Â Â__u8 protocol_id; > > Â Â Â Âstruct list_head qhead; /* mids waiting on reply from this server */ > > Â Â Â Âint midState; Â /* wish this were enum but can not pass to wait_event */ > > Â Â Â Âunsigned long when_alloc; Â/* when mid was created */ > > #ifdef CONFIG_CIFS_STATS2 > > Â Â Â Âunsigned long when_sent; /* time when smb send finished */ > > Â Â Â Âunsigned long when_received; /* when demux complete (taken off wire) */ > > #endif > > Â Â Â Âbool largeBuf:1; Â Â Â Â/* if valid response, is pointer to large buf */ > > Â Â Â Âvoid *resp_buf; Â Â Â Â /* response buffer */ > > Â Â Â Âmid_callback_t *callback; /* call completion callback */ > > Â Â Â Âvoid *callback_data; Â Â Â/* general purpose pointer for callback */ > > }; > > > > struct cifs_mid_q_entry { > > Â Â Â Âstruct mid_q_entry mid_q; > > Â Â Â Â__u16 mid; Â Â Â Â Â Â Â/* multiplex id */ > > Â Â Â Â__u32 sequence_number; Â/* for CIFS signing */ > > Â Â Â Â__u8 command; Â Â Â Â Â /* smb command code */ > > Â Â Â Â__u16 pid; Â Â Â Â Â Â Â/* process id */ > > Â Â Â Âbool multiRsp:1; Â Â Â Â/* multiple trans2 responses for one request Â*/ > > Â Â Â Âbool multiEnd:1; Â Â Â Â/* both received */ > > }; > > > > struct smb2_mid_q_entry { > > Â Â Â Âstruct mid_q_entry mid_q; > > Â Â Â Â__u64 mid; Â Â Â Â Â Â Â/* multiplex id(s), bigger for smb2 */ > > Â Â Â Â__le16 command; Â Â Â Â /* smb2 command code */ > > Â Â Â Â__u32 pid; Â Â Â Â Â Â Â/* process id - bigger for smb2 than cifs */ > > }; > > I think nested mid structures (around a base of common mid fields) > like the above is going to be the easiest way to handle the differences. > > I don't understand your PROTOCOL_ID #define though - we > identify smb2 vs. cifs via a bool in the tcp server info struct, > and presumably if we don't have access to the tcp server > info struct we would have to add a field in the base mid > (struct mid_q_entry) that indicates smb2 vs. cifs.. > Why do even that? Why not a common mid struct that's simply a little larger? Sure it means slightly more memory consumption, but we never have that many in flight. Something like: struct mid_q_entry { __u8 protocol_id; struct list_head qhead; /* mids waiting on reply from this server */ int midState; /* wish this were enum but can not pass to wait_event */ unsigned long when_alloc; /* when mid was created */ #ifdef CONFIG_CIFS_STATS2 unsigned long when_sent; /* time when smb send finished */ unsigned long when_received; /* when demux complete (taken off wire) */ #endif bool largeBuf:1; /* if valid response, is pointer to large buf */ void *resp_buf; /* response buffer */ mid_callback_t *callback; /* call completion callback */ void *callback_data; /* general purpose pointer for callback */ __u64 mid; /* multiplex id */ __le16 command; /* command code */ __u32 pid; /* process id */ __u32 sequence_number; /* for CIFS signing */ bool multiRsp:1; /* multiple trans2 responses for one request */ bool multiEnd:1; /* both received */ }; Using different structs here means that you need an entirely different set of code to deal with them. IMO, the memory savings (if any) isn't worth the duplicate code that'll be necessary. -- Jeff Layton <jlayton@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
