On Sat, 2011-07-30 at 09:42 -0700, Joe Eykholt wrote:
> On 7/30/11 7:54 AM, Nicholas A. Bellinger wrote:
> > On Wed, 2011-07-27 at 14:28 -0700, Kiran Patil wrote:
> >> From: Kiran Patil<kiran.patil@xxxxxxxxx>
> >>
> >> Problem: HW DDP context wasn;t invalidated in case of ABORTS, etc...
> >> This leads to the problem where memory pages which are used for DDP
> >> as user descriptor could get reused for some other purpose (such as to
> >> satisfy new memory allocation request either by kernel or user mode threads)
> >> and since HW DDP context was not invalidated, HW continue to write to
> >> those pages, hence causing memory corruption.
> >>
> >
> > Ouch...
> >
> >> Fix: Either on incoming ABORTS or due to exchange time out, allowed the
> >> target to cleanup HW DDP context if it was setup for respective ft_cmd.
> >> Added new function to perform this cleanup, furthur it can be enhanced
> >> for other cleanup activity.
> >>
> >> Additinal Notes: To avoid calling ddp_done from multiple places, composed
> >> the functionality in helper function "ft_invl_hw_context" and it is being
> >> called from multiple places. Cleaned up code in function "ft_recv_write_data"
> >> w.r.t DDP.
> >>
> >
> > Ok, along with fixing up the pr_debug / pr_err stuff to follow LIO
> > upstream, I did notice one thing below that I think is bogus.. Please
> > have a look:
> >
> >> Signed-off-by: Kiran Patil<kiran.patil@xxxxxxxxx>
> >> ---
> >>
> >> drivers/target/tcm_fc/tcm_fc.h | 5 ++
> >> drivers/target/tcm_fc/tfc_cmd.c | 1
> >> drivers/target/tcm_fc/tfc_io.c | 101 +++++++++++++++++++++++++--------------
> >> 3 files changed, 70 insertions(+), 37 deletions(-)
> >>
<SNIP>
> >
> > So the end result of the change to ft_recv_write_data() should look like
> > the following, yes..?
> >
> > /*
> > * Doesn't expect payload if DDP is setup. Payload
> > * is expected to be copied directly to user buffers
> > * due to DDP (Large Rx offload),
> > */
> > buf = fc_frame_payload_get(fp, 1);
> > if (cmd->was_ddp_setup&& buf) {
> > pr_err("%s: xid 0x%x, f_ctl 0x%x, cmd->sg %p, "
> > "cmd->sg_cnt 0x%x. DDP was setup"
> > " hence not expected to receive frame with "
> > "payload, Frame will be dropped if"
>
> Add space after "if"
>
> > "'Sequence Initiative' bit in f_ctl is"
>
> and after "is"
>
Changing up this cosmetic bit now, thanks for having a look.
> BTW, I like adding the space on the first line as opposed to the second.
> I'm not sure if there's a kernel standard for this.
Not AFAIK, but I think this falls under the personal style category. ;)
--nab
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
