On Wed, 2011-07-06 at 14:44 -0700, Joe Eykholt wrote:
> On 7/6/11 2:39 PM, Joe Eykholt wrote:
> > On 7/6/11 2:04 PM, Vasu Dev wrote:
> >> Drop the rx frame having xid with wrong cpu info
> >> or received with xid not matching to our xid.
> >>
> >> Not dropping such frame is causing panic as
> >> that causes accessing data struct beyond their
> >> bounds.
> >
> > What kind of frame was it that caused this?
> > It might be better to deliver it to one of the valid
> > CPUs instead of just dropping it. That's what it once did.
>
> By the way, I noticed that in this code in fcoe.c:
>
> 1363 cpu = smp_processor_id();
> 1364
> 1365 if ((fh->fh_type == FC_TYPE_FCP) &&
> 1366 (ntohs(fh->fh_rx_id) == FC_XID_UNKNOWN)) {
> 1367 do {
> 1368 cpu = fcoe_select_cpu(cpu);
> 1369 } while (!cpu_online(cpu));
> 1370 } else if ((fh->fh_type == FC_TYPE_FCP) &&
> 1371 (ntohs(fh->fh_rx_id) != FC_XID_UNKNOWN)) {
> 1372 cpu = ntohs(fh->fh_rx_id) & fc_cpu_mask;
> 1373 } else
> 1374 cpu = smp_processor_id();
>
> Note that both lines 1363 and 1374 set the same value in cpu, so
> one of these might be eliminated.
As I started cleaning up this redundant cpu init, I found some more
cleanup required in selecting CPUs for incoming request.
- Have selection for all types of frames, not just FCP
- Don't check above cpu_online on line #1369, not required as down below
offline cpu case is handled.
- Also simplify fcoe_select_cpu() by removing unnecessary checks to skip
curr_cpu, I came to know that also fixes possibly infinite loop in case
of curr_cpu is the only cpu while iterating in the loop.
I'm pasting the new cleanup patch below for RFC and will get same out
after its testing, just replying along this patch discussion.
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index c971fc6..a57c29e 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1270,30 +1270,20 @@ static int fcoe_cpu_callback(struct
notifier_block *nfb,
/**
* fcoe_select_cpu() - Selects CPU to handle post-processing of
incoming
* command.
- * @curr_cpu: CPU which received request
*
- * This routine selects next CPU based on cpumask.
+ * This routine selects next CPU based on cpumask to distribute
+ * incoming requests in round robin.
*
- * Returns: int (CPU number). Caller to verify if returned CPU is
online or not.
+ * Returns: int CPU number
*/
-static unsigned int fcoe_select_cpu(unsigned int curr_cpu)
+static inline unsigned int fcoe_select_cpu(void)
{
static unsigned int selected_cpu;
- if (num_online_cpus() == 1)
- return curr_cpu;
- /*
- * Doing following check, to skip "curr_cpu (smp_processor_id)"
- * from selection of CPU is intentional. This is to avoid same
CPU
- * doing post-processing of command. "curr_cpu" to just receive
- * incoming request in case where rx_id is UNKNOWN and all other
- * CPU to actually process the command(s)
- */
- do {
- selected_cpu = cpumask_next(selected_cpu,
cpu_online_mask);
- if (selected_cpu >= nr_cpu_ids)
- selected_cpu = cpumask_first(cpu_online_mask);
- } while (selected_cpu == curr_cpu);
+ selected_cpu = cpumask_next(selected_cpu, cpu_online_mask);
+ if (selected_cpu >= nr_cpu_ids)
+ selected_cpu = cpumask_first(cpu_online_mask);
+
return selected_cpu;
}
@@ -1368,23 +1358,16 @@ int fcoe_rcv(struct sk_buff *skb, struct
net_device *netdev,
* In case the incoming frame's exchange is originated from
* the initiator, then received frame's exchange id is ANDed
* with fc_cpu_mask bits to get the same cpu on which exchange
- * was originated, otherwise just use the current cpu.
+ * was originated, otherwise select cpu using rx exchange id
+ * or fcoe_select_cpu().
*/
if (ntoh24(fh->fh_f_ctl) & FC_FC_EX_CTX)
cpu = ntohs(fh->fh_ox_id) & fc_cpu_mask;
else {
- cpu = smp_processor_id();
-
- if ((fh->fh_type == FC_TYPE_FCP) &&
- (ntohs(fh->fh_rx_id) == FC_XID_UNKNOWN)) {
- do {
- cpu = fcoe_select_cpu(cpu);
- } while (!cpu_online(cpu));
- } else if ((fh->fh_type == FC_TYPE_FCP) &&
- (ntohs(fh->fh_rx_id) != FC_XID_UNKNOWN)) {
+ if (ntohs(fh->fh_rx_id) == FC_XID_UNKNOWN)
+ cpu = fcoe_select_cpu();
+ else
cpu = ntohs(fh->fh_rx_id) & fc_cpu_mask;
- } else
- cpu = smp_processor_id();
}
if (cpu >= nr_cpu_ids)
_______________________________________________
devel mailing list
[email protected]
https://lists.open-fcoe.org/mailman/listinfo/devel
