On Fri, 15 Apr 2011 08:38:59 -0500 Steve French <[email protected]> wrote:
> On Fri, Apr 15, 2011 at 5:45 AM, Pavel Shilovsky <[email protected]> wrote: > > 2011/4/8 Steve French <[email protected]>: > >> 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 [email protected] > >> 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 <[email protected]> -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
