On Mon, Jun 10, 2013 at 04:28:14PM -0700, Robert Love wrote: > Logging in the timer arm routine doesn't provide any > context to the user who is debugging. Move the debug > logging statement to the caller such that the context > may be presented in the log statement. > > Signed-off-by: Robert Love <[email protected]> > Tested-by: Jack Morgan<[email protected]> > --- > drivers/scsi/libfc/fc_exch.c | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c > index 0316cbf..1707cbb 100644 > --- a/drivers/scsi/libfc/fc_exch.c > +++ b/drivers/scsi/libfc/fc_exch.c > @@ -360,8 +360,6 @@ static inline void fc_exch_timer_set_locked(struct > fc_exch *ep, > if (ep->state & (FC_EX_RST_CLEANUP | FC_EX_DONE)) > return; > > - FC_EXCH_DBG(ep, "Exchange timer armed : %d msecs\n", timer_msec); > - > if (queue_delayed_work(fc_exch_workqueue, &ep->timeout_work, > msecs_to_jiffies(timer_msec))) > fc_exch_hold(ep); /* hold for timer */ > @@ -612,8 +610,10 @@ static int fc_exch_abort_locked(struct fc_exch *ep, > return -ENOMEM; > > ep->esb_stat |= ESB_ST_SEQ_INIT | ESB_ST_ABNORMAL; > - if (timer_msec) > + if (timer_msec) { > + FC_EXCH_DBG(ep, "Exchange timer armed (exchange being aborted): > %d msecs\n", timer_msec); > fc_exch_timer_set_locked(ep, timer_msec); > + } > > /* > * If not logged into the fabric, don't send ABTS but leave > @@ -1288,6 +1288,7 @@ static void fc_exch_recv_abts(struct fc_exch *ep, > struct fc_frame *rx_fp) > if (!(ep->esb_stat & ESB_ST_REC_QUAL)) > fc_exch_hold(ep); /* hold for REC_QUAL */ > ep->esb_stat |= ESB_ST_ABNORMAL | ESB_ST_REC_QUAL; > + FC_EXCH_DBG(ep, "Exchange timer armed (reveived ABTS): %d msecs\n", > timer_msec); > fc_exch_timer_set_locked(ep, ep->r_a_tov); > > fp = fc_frame_alloc(ep->lp, sizeof(*ap)); > @@ -1610,8 +1611,10 @@ static void fc_exch_abts_resp(struct fc_exch *ep, > struct fc_frame *fp) > else > fc_frame_free(fp); > > - if (has_rec) > + if (has_rec) { > + FC_EXCH_DBG(ep, "Exchange timer armed (received ABTS response, > has REC): %d msecs\n", timer_msec); > fc_exch_timer_set(ep, ep->r_a_tov); > + } > > } > > @@ -2024,8 +2027,10 @@ static struct fc_seq *fc_exch_seq_send(struct fc_lport > *lport, > if (unlikely(lport->tt.frame_send(lport, fp))) > goto err; > > - if (timer_msec) > + if (timer_msec) { > + FC_EXCH_DBG(ep, "Exchange timer armed (sending exch): %d > msecs\n", timer_msec); > fc_exch_timer_set_locked(ep, timer_msec); > + } > ep->f_ctl &= ~FC_FC_FIRST_SEQ; /* not first seq */ > > if (ep->f_ctl & FC_FC_SEQ_INIT) > @@ -2090,6 +2095,7 @@ retry: > return; > } > ep->esb_stat |= ESB_ST_REC_QUAL; > + FC_EXCH_DBG(ep, "Exchange timer armed (sending RRQ): %d msecs\n", > timer_msec); > fc_exch_timer_set_locked(ep, ep->r_a_tov); > spin_unlock_bh(&ep->ex_lock); > } > > _______________________________________________ > fcoe-devel mailing list > [email protected] > http://lists.open-fcoe.org/mailman/listinfo/fcoe-devel >
I know its just a few call sites now, but to prevent future fan out (and people forgetting to add a printk with the call to fc_exch_timer_set_locked), would it be better to keep the FC_EXCH_DBG call in the function, and use builtin_return_address and lookup_symbol_name() to resolve the context issue? Neil _______________________________________________ fcoe-devel mailing list [email protected] http://lists.open-fcoe.org/mailman/listinfo/fcoe-devel
