Re: [PATCH] [CIFS] Add structure definitions for SMB2 PDUs

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

 



On Fri, 25 Feb 2011 23:25:08 -0600
Steve French <smfrench@xxxxxxxxx> wrote:
 +
> +/* one of these for every pending SMB2 request to the server */
> +struct smb2_mid_entry {
> +	struct list_head qhead;	/* mids waiting on reply from this server */
> +	__u64 mid;		/* multiplex id(s) */
> +	__u16 pid;		/* process id */
> +	__u32 sequence_number;	/* for signing */ /* BB check if needed */
> +	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
> +	struct task_struct *tsk;	/* task waiting for response */
> +	struct smb2_hdr *resp_buf;	/* response buffer */
> +	char **pagebuf_list;	        /* response buffer */
> +	int num_pages;
> +	int mid_state;	/* wish this were enum but can not pass to wait_event */
> +	__le16 command;	/* smb command code */
> +	bool async_resp_rcvd:1; /* if server has responded with interim resp */
> +	bool large_buf:1;	/* if valid response, is pointer to large buf */
> +	bool is_kmap_buf:1;
> +/*	bool multi_rsp:1; BB do we have to account for something in SMB2 like
> +	we saw multiple trans2 responses for one request (possible in CIFS) */
> +	/* Async things */
> +	__u64 *mid_list;	/* multiplex id(s) */
> +	int *mid_state_list;
> +	short int *large_buf_list;
> +	unsigned int num_mid;
> +	unsigned int act_num_mid;
> +	unsigned int num_received;
> +	unsigned int cur_id;
> +	struct smb2_hdr **resp_buf_list;	/* response buffer */
> +	__le16 *command_list;
> +	bool async:1;
> +	bool complex_mid:1; /* complex entry - consists of several messages */
> +	int result;
> +	unsigned long last_rsp_time;
> +	int (*callback)(struct smb2_mid_entry * , void *);
> +	void *callback_data;
> +};
> +
>

Holy cow, this thing is a porker.

Can we eliminate some of these fields? For instance, you have a
callback and callback_data field like in the cifs variant, but you also
have a tsk pointer. That should no longer be needed. There also seem to
be a lot of fields that are not used in any of the code that has yet
been merged. Much of it is really unclear.

This seems like a real red flag to me. Dumping code into the tree like
this en-masse is a recipe for bloat and waste. Adding structs for
protocol definitions is one thing, adding things like this that seem to
have no rhyme or reason is quite another.

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


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

  Powered by Linux