Re: [PATCH] sync_smb2_mid_result

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

 



On Wed, 23 Mar 2011 13:02:51 -0500
Steve French <smfrench@xxxxxxxxx> wrote:

> On Wed, Mar 23, 2011 at 12:58 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> > On Wed, 23 Mar 2011 12:42:26 -0500
> > Steve French <smfrench@xxxxxxxxx> wrote:
> >
> >> 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)
> >>
> >
> > No. I mean use the same struct for both, expanding fields in the "old"
> > mid_q_entry struct as needed. For the "mid" field itself, for instance
> > you could grow it to a u64 and just plan to not use the upper bits for
> > smb1. Other fields could be handed similarly.
> >
> > A common struct here means that you can reuse more code.
> 
> We could make it a union presumably, but it means more
> casts as the sizes of fields are different, and smb2 doesn't
> need some of cifs fields, and smb2 will need at least 5
> (but probably 10) more fields such as for "async response
> received" and for chains of commands.  Overall that will
> cause the size of the structure for cifs to grow at least
> 25% (probably more).
> 
> 

Maybe you misunderstand me.

The struct is *exactly the same* aside from a few small differences. It
doesn't even need to be a union, IMO. That would let you use a lot of
the same code in transport.c for smb2.

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