RE: [PATCH net-next 2/2] r8152: adjust rtl_start_rx

[Hayes Wang]

David Miller [mailto:davem@xxxxxxxxxxxxx] 
> Sent: Wednesday, November 12, 2014 10:51 AM
[...]
> Ok, but if we are looping here in rtl_start_rx() and r8152_submit_rx()
> fails due to a memory allocation failure, there is nothing which is
> going to make such a memory allocation succeed in the next iteration
> of the loop.
> 
> Unless you can prove that often it can succeed after an initial
> failure, this is just wasted work and in fact making it take longer
> for the system to reclaim memory when under pressure because these
> extra iterations are completely wasted cpu work.

How about that when a error occurs, add the remaining rx
to the list without submission? Then, the remianing rx
could be re-submitted later, and the rtl_start_rx() could
be completed as soon as possible.

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 0a30fd3..3273e3d 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1991,14 +1991,35 @@ static void rxdy_gated_en(struct r8152 *tp, bool enable)
 
 static int rtl_start_rx(struct r8152 *tp)
 {
+	struct list_head rx_queue;
 	int i, ret = 0;
 
 	INIT_LIST_HEAD(&tp->rx_done);
 	for (i = 0; i < RTL8152_MAX_RX; i++) {
 		INIT_LIST_HEAD(&tp->rx_info[i].list);
 		ret = r8152_submit_rx(tp, &tp->rx_info[i], GFP_KERNEL);
-		if (ret)
+		if (ret) {
+			i++;
 			break;
+		}
+	}
+
+	INIT_LIST_HEAD(&rx_queue);
+	for (; i < RTL8152_MAX_RX; i++) {
+		struct rx_agg *agg = &tp->rx_info[i];
+		struct urb *urb = agg->urb;
+
+		INIT_LIST_HEAD(&agg->list);
+		urb->actual_length = 0;
+		list_add_tail(&agg->list, &rx_queue);
+	}
+
+	if (!list_empty(&rx_queue)) {
+		unsigned long flags;
+
+		spin_lock_irqsave(&tp->rx_lock, flags);
+		list_splice_tail(&rx_queue, &tp->rx_done);
+		spin_unlock_irqrestore(&tp->rx_lock, flags);
 	}
 
 	return ret;
 
Best Regards,
Hayes
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/