On Wed, 2010-10-06 at 12:17 +0800, Hillf Danton wrote: > On Tue, Oct 5, 2010 at 7:16 AM, Robert Love <[email protected]> wrote: > > On Sun, 2010-09-26 at 07:06 -0700, Hillf Danton wrote: > >> The operation related to fr_seq(fp) seems unnecessary, since it is > >> also modified in fc_fill_hdr(). > >> Or fr_seq(fp) should be computed after calling fc_fill_hdr(). > >> > >> Signed-off-by: Hillf Danton <[email protected]> > >> --- > >> > >> --- o/linux-2.6.36-rc4/drivers/scsi/libfc/fc_libfc.c 2010-09-13 > >> 07:07:38.000000000 +0800 > >> +++ m/linux-2.6.36-rc4/drivers/scsi/libfc/fc_libfc.c 2010-09-26 > >> 20:54:14.000000000 +0800 > >> @@ -202,11 +202,6 @@ EXPORT_SYMBOL(fc_fill_hdr); > >> void fc_fill_reply_hdr(struct fc_frame *fp, const struct fc_frame *in_fp, > >> enum fc_rctl r_ctl, u32 parm_offset) > >> { > >> - struct fc_seq *sp; > >> - > >> - sp = fr_seq(in_fp); > >> - if (sp) > >> - fr_seq(fp) = fr_dev(in_fp)->tt.seq_start_next(sp);
This assignment makes it so the sequence that is in in_fp is the same one in fp, so the old frame and the new frame actually share the same sequence. > >> fc_fill_hdr(fp, in_fp, r_ctl, FC_FCTL_RESP, 0, parm_offset); > >> } > >> EXPORT_SYMBOL(fc_fill_reply_hdr); > > > > Hi Hillf, > > > > I don't think you can remove this code. In the default case > > lport->tt.seq_start_next() will point to fc_seq_start_next() and will > > increment the seq_id. It will then be inserted into the new frame header > > in fc_fill_hdr(). > > It seems what seq_start_next does is not inserted in fc_fill_hdr() but > replaced with fr_seq(in_fp), if fr_seq(in_fp) is not false. > Since the sequence is shared between fp and in_fp there is no replacement. The sequence with the incremented seq_id is used by the new fp. Also, keep in mind that fc_seq_start_next()/fc_seq_alloc() doesn't really _allocate_ a new sequence. It takes the exchange from the sequence (using container_of) and then updates the sequence within that exchange (the same sequence we started with). This confused me too when I was reviewing your patch. I think there is room for improvement to make the code more straight-forward. At least a comment would be nice. //Rob _______________________________________________ devel mailing list [email protected] http://www.open-fcoe.org/mailman/listinfo/devel
