Re: [PATCH] sync_smb2_mid_result

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 23, 2011 at 11:06 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> On Sat, 12 Mar 2011 19:55:55 -0600
> Steve French <smfrench@xxxxxxxxx> wrote:
>
>> Previously CIFS added a function, sync_mid_result, to better handle
>> certain error conditions.
>> This patch adds the equivalent for SMB2 mids (they are used by smb2_sendrcv2
>> which is added in the next patch)

> Now that you've commented out all of the unneeded fields from the
> struct smb2_mid_entry, it's clear that the smb2_mid_entry is a subset
> of mid_q_entry. Some of the mid_q_entry fields might need to be
> expanded (the mid number itself for example), but that hardly seems to
> warrant a completely new struct and codebase for this.
>
> Can you explain why we need all of this code duplication when it now
> seems fairly clear that much of the existing code could be repurposed
> for SMB2? This approach is going to vastly increase the maintenance
> burden and I don't see that we gain anything from it.

Do you mean changing the cifs mid from its current single struct to a
nested mid_common which can be included in cifs and smb2 specific mids
(or made into a union)

struct mid_q_entry {
	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 */
	mid_callback_t *callback; /* call completion callback */
	void *callback_data;	  /* general purpose pointer for callback */
	__u16 mid;		/* multiplex id */
	__u32 sequence_number;  /* for CIFS signing */
	__u8 command;		/* smb command code */
	__u16 pid;		/* process id */
	struct smb_hdr *resp_buf;	/* response buffer */
	bool multiRsp:1;	/* multiple trans2 responses for one request  */
	bool multiEnd:1;	/* both received */
};

to
struct mid_q_entry { /* common fields for smb2 and cifs mids */
	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 */
	mid_callback_t *callback; /* call completion callback */
	void *callback_data;	  /* general purpose pointer for callback */
}

and then cifs mids would look like:
struct cifs_mid_entry {
        struct mid_q_entry; /* common fields */
	__u16 mid;		/* multiplex id */
	__u32 sequence_number;  /* for CIFS signing */
	__u8 command;		/* smb command code */
	__u16 pid;		/* process id */
	struct smb_hdr *resp_buf;	/* response buffer */
	bool multiRsp:1;	/* multiple trans2 responses for one request  */
	bool multiEnd:1;	/* both received */
};

and smb2_mids would look like:
struct smb2_mid_entry {
        struct mid_q_entry; /* common fields */
	__u64 mid;		/* multiplex id(s), bigger for smb2 */
	__le16 command;		/* smb2 command code */
	__u32 pid;		/* process id - bigger for smb2 than cifs */
	struct smb2_hdr *resp_buf;	/* response buffer */
	/* More than 10 additional fields were used for other smb2 unique
        features that have not been ported to the new cifs.ko structure
        yet including chains of related commands for handling
        compound smb2 operations,  and for asynchronous smb2_writepages
support have been temporarily
	removed from the port and will be reenabled as that gets merged in */
};







-- 
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


[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux