All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/9] s390/qeth: updates 2021-06-11
@ 2021-06-11  7:33 Julian Wiedmann
  2021-06-11  7:33 ` [PATCH net-next 1/9] s390/qeth: count TX completion interrupts Julian Wiedmann
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Julian Wiedmann @ 2021-06-11  7:33 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: linux-netdev, linux-s390, Heiko Carstens, Karsten Graul, Julian Wiedmann

Hi Dave & Jakub,

please apply the following patch series for qeth to netdev's net-next tree.

This enables TX NAPI for those devices that didn't use it previously, so
that we can eventually rip out the qdio layer's internal interrupt
machinery.

Other than that it's just the normal mix of minor improvements and
cleanups.

Thanks,
Julian

Alexandra Winter (1):
  s390/qeth: Consider dependency on SWITCHDEV module

Julian Wiedmann (8):
  s390/qeth: count TX completion interrupts
  s390/qeth: also use TX NAPI for non-IQD devices
  s390/qeth: unify the tracking of active cmds on ccw device
  s390/qeth: use ethtool_sprintf()
  s390/qeth: consolidate completion of pending TX buffers
  s390/qeth: remove QAOB's pointer to its TX buffer
  s390/qeth: remove TX buffer's pointer to its queue
  s390/qeth: shrink TX buffer struct

 arch/s390/include/asm/qdio.h      |   4 +-
 drivers/s390/net/qeth_core.h      |  42 ++--
 drivers/s390/net/qeth_core_main.c | 349 ++++++++++++------------------
 drivers/s390/net/qeth_ethtool.c   |   7 +-
 drivers/s390/net/qeth_l2_main.c   |  12 +-
 5 files changed, 181 insertions(+), 233 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH net-next 1/9] s390/qeth: count TX completion interrupts
  2021-06-11  7:33 [PATCH net-next 0/9] s390/qeth: updates 2021-06-11 Julian Wiedmann
@ 2021-06-11  7:33 ` Julian Wiedmann
  2021-06-11  7:33 ` [PATCH net-next 2/9] s390/qeth: also use TX NAPI for non-IQD devices Julian Wiedmann
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Julian Wiedmann @ 2021-06-11  7:33 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: linux-netdev, linux-s390, Heiko Carstens, Karsten Graul, Julian Wiedmann

While the qdio layer already tracks the number of HW interrupts for a
device, there's value in understanding how many of them have been
raised due to our TX completion logic.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_core.h      | 1 +
 drivers/s390/net/qeth_core_main.c | 4 +++-
 drivers/s390/net/qeth_ethtool.c   | 1 +
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index fd9b869d278e..3a49ef8dd906 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -483,6 +483,7 @@ struct qeth_out_q_stats {
 	u64 stopped;
 	u64 doorbell;
 	u64 coal_frames;
+	u64 completion_irq;
 	u64 completion_yield;
 	u64 completion_timer;
 
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index a1f08e9aa064..9085f22ca34c 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -1400,8 +1400,10 @@ static void qeth_clear_output_buffer(struct qeth_qdio_out_q *queue,
 	int i;
 
 	/* is PCI flag set on buffer? */
-	if (buf->buffer->element[0].sflags & SBAL_SFLAGS0_PCI_REQ)
+	if (buf->buffer->element[0].sflags & SBAL_SFLAGS0_PCI_REQ) {
 		atomic_dec(&queue->set_pci_flags_count);
+		QETH_TXQ_STAT_INC(queue, completion_irq);
+	}
 
 	qeth_tx_complete_buf(buf, error, budget);
 
diff --git a/drivers/s390/net/qeth_ethtool.c b/drivers/s390/net/qeth_ethtool.c
index 3a51bbff0ffe..190dac2065df 100644
--- a/drivers/s390/net/qeth_ethtool.c
+++ b/drivers/s390/net/qeth_ethtool.c
@@ -41,6 +41,7 @@ static const struct qeth_stats txq_stats[] = {
 	QETH_TXQ_STAT("Queue stopped", stopped),
 	QETH_TXQ_STAT("Doorbell", doorbell),
 	QETH_TXQ_STAT("IRQ for frames", coal_frames),
+	QETH_TXQ_STAT("Completion IRQ", completion_irq),
 	QETH_TXQ_STAT("Completion yield", completion_yield),
 	QETH_TXQ_STAT("Completion timer", completion_timer),
 };
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next 2/9] s390/qeth: also use TX NAPI for non-IQD devices
  2021-06-11  7:33 [PATCH net-next 0/9] s390/qeth: updates 2021-06-11 Julian Wiedmann
  2021-06-11  7:33 ` [PATCH net-next 1/9] s390/qeth: count TX completion interrupts Julian Wiedmann
@ 2021-06-11  7:33 ` Julian Wiedmann
  2021-06-11  7:33 ` [PATCH net-next 3/9] s390/qeth: unify the tracking of active cmds on ccw device Julian Wiedmann
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Julian Wiedmann @ 2021-06-11  7:33 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: linux-netdev, linux-s390, Heiko Carstens, Karsten Graul, Julian Wiedmann

Set scan_threshold = 0 to opt out from the qdio layer's internal tasklet
& timer mechanism for TX completions, and replace it with the TX NAPI
infrastructure that qeth already uses for IQD devices. This avoids the
fragile logic in qdio_check_output_queue(), enables tighter integration
and gives us more tuning options via ethtool in the future.

For now we continue to apply the same policy as the qdio layer:
scan for completions if 32 TX buffers are in use, or after 1 sec.
A re-scan is done after 10 sec, but only if no TX interrupt is pending.

With scan_threshold = 0 we no longer get TX completion scans from
within qdio_get_next_buffers(). So trigger these manually in qeth_poll()
and in the RX path switch to the equivalent qdio_inspect_queue().

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_core.h      |   6 ++
 drivers/s390/net/qeth_core_main.c | 148 +++++++++++++++---------------
 2 files changed, 79 insertions(+), 75 deletions(-)

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index 3a49ef8dd906..4d29801bcf41 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -527,6 +527,7 @@ struct qeth_qdio_out_q {
 
 	unsigned int coalesce_usecs;
 	unsigned int max_coalesced_frames;
+	unsigned int rescan_usecs;
 };
 
 #define qeth_for_each_output_queue(card, q, i)		\
@@ -887,6 +888,11 @@ static inline bool qeth_card_hw_is_reachable(struct qeth_card *card)
 	return card->state == CARD_STATE_SOFTSETUP;
 }
 
+static inline bool qeth_use_tx_irqs(struct qeth_card *card)
+{
+	return !IS_IQD(card);
+}
+
 static inline void qeth_unlock_channel(struct qeth_card *card,
 				       struct qeth_channel *channel)
 {
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 9085f22ca34c..f22f223a4a6c 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -2665,8 +2665,15 @@ static int qeth_alloc_qdio_queues(struct qeth_card *card)
 		INIT_LIST_HEAD(&queue->pending_bufs);
 		spin_lock_init(&queue->lock);
 		timer_setup(&queue->timer, qeth_tx_completion_timer, 0);
-		queue->coalesce_usecs = QETH_TX_COALESCE_USECS;
-		queue->max_coalesced_frames = QETH_TX_MAX_COALESCED_FRAMES;
+		if (IS_IQD(card)) {
+			queue->coalesce_usecs = QETH_TX_COALESCE_USECS;
+			queue->max_coalesced_frames = QETH_TX_MAX_COALESCED_FRAMES;
+			queue->rescan_usecs = QETH_TX_TIMER_USECS;
+		} else {
+			queue->coalesce_usecs = USEC_PER_SEC;
+			queue->max_coalesced_frames = 0;
+			queue->rescan_usecs = 10 * USEC_PER_SEC;
+		}
 		queue->priority = QETH_QIB_PQUE_PRIO_DEFAULT;
 	}
 
@@ -3603,8 +3610,8 @@ static void qeth_flush_buffers(struct qeth_qdio_out_q *queue, int index,
 			       int count)
 {
 	struct qeth_qdio_out_buffer *buf = queue->bufs[index];
-	unsigned int qdio_flags = QDIO_FLAG_SYNC_OUTPUT;
 	struct qeth_card *card = queue->card;
+	unsigned int frames, usecs;
 	struct qaob *aob = NULL;
 	int rc;
 	int i;
@@ -3660,14 +3667,11 @@ static void qeth_flush_buffers(struct qeth_qdio_out_q *queue, int index,
 				buf->buffer->element[0].sflags |= SBAL_SFLAGS0_PCI_REQ;
 			}
 		}
-
-		if (atomic_read(&queue->set_pci_flags_count))
-			qdio_flags |= QDIO_FLAG_PCI_OUT;
 	}
 
 	QETH_TXQ_STAT_INC(queue, doorbell);
-	rc = do_QDIO(CARD_DDEV(card), qdio_flags, queue->queue_no, index, count,
-		     aob);
+	rc = do_QDIO(CARD_DDEV(card), QDIO_FLAG_SYNC_OUTPUT, queue->queue_no,
+		     index, count, aob);
 
 	switch (rc) {
 	case 0:
@@ -3675,17 +3679,20 @@ static void qeth_flush_buffers(struct qeth_qdio_out_q *queue, int index,
 		/* ignore temporary SIGA errors without busy condition */
 
 		/* Fake the TX completion interrupt: */
-		if (IS_IQD(card)) {
-			unsigned int frames = READ_ONCE(queue->max_coalesced_frames);
-			unsigned int usecs = READ_ONCE(queue->coalesce_usecs);
+		frames = READ_ONCE(queue->max_coalesced_frames);
+		usecs = READ_ONCE(queue->coalesce_usecs);
 
-			if (frames && queue->coalesced_frames >= frames) {
-				napi_schedule(&queue->napi);
-				queue->coalesced_frames = 0;
-				QETH_TXQ_STAT_INC(queue, coal_frames);
-			} else if (usecs) {
-				qeth_tx_arm_timer(queue, usecs);
-			}
+		if (frames && queue->coalesced_frames >= frames) {
+			napi_schedule(&queue->napi);
+			queue->coalesced_frames = 0;
+			QETH_TXQ_STAT_INC(queue, coal_frames);
+		} else if (qeth_use_tx_irqs(card) &&
+			   atomic_read(&queue->used_buffers) >= 32) {
+			/* Old behaviour carried over from the qdio layer: */
+			napi_schedule(&queue->napi);
+			QETH_TXQ_STAT_INC(queue, coal_frames);
+		} else if (usecs) {
+			qeth_tx_arm_timer(queue, usecs);
 		}
 
 		break;
@@ -3833,36 +3840,14 @@ static void qeth_qdio_output_handler(struct ccw_device *ccwdev,
 				     unsigned long card_ptr)
 {
 	struct qeth_card *card        = (struct qeth_card *) card_ptr;
-	struct qeth_qdio_out_q *queue = card->qdio.out_qs[__queue];
 	struct net_device *dev = card->dev;
-	struct netdev_queue *txq;
-	int i;
 
 	QETH_CARD_TEXT(card, 6, "qdouhdl");
 	if (qdio_error & QDIO_ERROR_FATAL) {
 		QETH_CARD_TEXT(card, 2, "achkcond");
 		netif_tx_stop_all_queues(dev);
 		qeth_schedule_recovery(card);
-		return;
-	}
-
-	for (i = first_element; i < (first_element + count); ++i) {
-		struct qeth_qdio_out_buffer *buf = queue->bufs[QDIO_BUFNR(i)];
-
-		qeth_handle_send_error(card, buf, qdio_error);
-		qeth_clear_output_buffer(queue, buf, qdio_error, 0);
 	}
-
-	atomic_sub(count, &queue->used_buffers);
-	qeth_check_outbound_queue(queue);
-
-	txq = netdev_get_tx_queue(dev, __queue);
-	/* xmit may have observed the full-condition, but not yet stopped the
-	 * txq. In which case the code below won't trigger. So before returning,
-	 * xmit will re-check the txq's fill level and wake it up if needed.
-	 */
-	if (netif_tx_queue_stopped(txq) && !qeth_out_queue_is_full(queue))
-		netif_tx_wake_queue(txq);
 }
 
 /**
@@ -5258,7 +5243,6 @@ static int qeth_qdio_establish(struct qeth_card *card)
 	init_data.int_parm               = (unsigned long) card;
 	init_data.input_sbal_addr_array  = in_sbal_ptrs;
 	init_data.output_sbal_addr_array = out_sbal_ptrs;
-	init_data.scan_threshold	 = IS_IQD(card) ? 0 : 32;
 
 	if (atomic_cmpxchg(&card->qdio.state, QETH_QDIO_ALLOCATED,
 		QETH_QDIO_ESTABLISHED) == QETH_QDIO_ALLOCATED) {
@@ -5958,9 +5942,10 @@ static unsigned int qeth_rx_poll(struct qeth_card *card, int budget)
 		/* Fetch completed RX buffers: */
 		if (!card->rx.b_count) {
 			card->rx.qdio_err = 0;
-			card->rx.b_count = qdio_get_next_buffers(
-				card->data.ccwdev, 0, &card->rx.b_index,
-				&card->rx.qdio_err);
+			card->rx.b_count = qdio_inspect_queue(CARD_DDEV(card),
+							      0, true,
+							      &card->rx.b_index,
+							      &card->rx.qdio_err);
 			if (card->rx.b_count <= 0) {
 				card->rx.b_count = 0;
 				break;
@@ -6024,6 +6009,16 @@ int qeth_poll(struct napi_struct *napi, int budget)
 
 	work_done = qeth_rx_poll(card, budget);
 
+	if (qeth_use_tx_irqs(card)) {
+		struct qeth_qdio_out_q *queue;
+		unsigned int i;
+
+		qeth_for_each_output_queue(card, queue, i) {
+			if (!qeth_out_queue_is_empty(queue))
+				napi_schedule(&queue->napi);
+		}
+	}
+
 	if (card->options.cq == QETH_CQ_ENABLED)
 		qeth_cq_poll(card);
 
@@ -6140,7 +6135,10 @@ static int qeth_tx_poll(struct napi_struct *napi, int budget)
 	unsigned int work_done = 0;
 	struct netdev_queue *txq;
 
-	txq = netdev_get_tx_queue(dev, qeth_iqd_translate_txq(dev, queue_no));
+	if (IS_IQD(card))
+		txq = netdev_get_tx_queue(dev, qeth_iqd_translate_txq(dev, queue_no));
+	else
+		txq = netdev_get_tx_queue(dev, queue_no);
 
 	while (1) {
 		unsigned int start, error, i;
@@ -6167,8 +6165,9 @@ static int qeth_tx_poll(struct napi_struct *napi, int budget)
 					       &start, &error);
 		if (completed <= 0) {
 			/* Ensure we see TX completion for pending work: */
-			if (napi_complete_done(napi, 0))
-				qeth_tx_arm_timer(queue, QETH_TX_TIMER_USECS);
+			if (napi_complete_done(napi, 0) &&
+			    !atomic_read(&queue->set_pci_flags_count))
+				qeth_tx_arm_timer(queue, queue->rescan_usecs);
 			return 0;
 		}
 
@@ -6181,12 +6180,19 @@ static int qeth_tx_poll(struct napi_struct *napi, int budget)
 			bytes += buffer->bytes;
 
 			qeth_handle_send_error(card, buffer, error);
-			qeth_iqd_tx_complete(queue, bidx, error, budget);
+			if (IS_IQD(card))
+				qeth_iqd_tx_complete(queue, bidx, error, budget);
+			else
+				qeth_clear_output_buffer(queue, buffer, error,
+							 budget);
 		}
 
-		netdev_tx_completed_queue(txq, packets, bytes);
 		atomic_sub(completed, &queue->used_buffers);
 		work_done += completed;
+		if (IS_IQD(card))
+			netdev_tx_completed_queue(txq, packets, bytes);
+		else
+			qeth_check_outbound_queue(queue);
 
 		/* xmit may have observed the full-condition, but not yet
 		 * stopped the txq. In which case the code below won't trigger.
@@ -7230,6 +7236,8 @@ EXPORT_SYMBOL_GPL(qeth_iqd_select_queue);
 int qeth_open(struct net_device *dev)
 {
 	struct qeth_card *card = dev->ml_priv;
+	struct qeth_qdio_out_q *queue;
+	unsigned int i;
 
 	QETH_CARD_TEXT(card, 4, "qethopen");
 
@@ -7237,16 +7245,11 @@ int qeth_open(struct net_device *dev)
 	netif_tx_start_all_queues(dev);
 
 	local_bh_disable();
-	if (IS_IQD(card)) {
-		struct qeth_qdio_out_q *queue;
-		unsigned int i;
-
-		qeth_for_each_output_queue(card, queue, i) {
-			netif_tx_napi_add(dev, &queue->napi, qeth_tx_poll,
-					  QETH_NAPI_WEIGHT);
-			napi_enable(&queue->napi);
-			napi_schedule(&queue->napi);
-		}
+	qeth_for_each_output_queue(card, queue, i) {
+		netif_tx_napi_add(dev, &queue->napi, qeth_tx_poll,
+				  QETH_NAPI_WEIGHT);
+		napi_enable(&queue->napi);
+		napi_schedule(&queue->napi);
 	}
 
 	napi_enable(&card->napi);
@@ -7261,6 +7264,8 @@ EXPORT_SYMBOL_GPL(qeth_open);
 int qeth_stop(struct net_device *dev)
 {
 	struct qeth_card *card = dev->ml_priv;
+	struct qeth_qdio_out_q *queue;
+	unsigned int i;
 
 	QETH_CARD_TEXT(card, 4, "qethstop");
 
@@ -7268,24 +7273,17 @@ int qeth_stop(struct net_device *dev)
 	cancel_delayed_work_sync(&card->buffer_reclaim_work);
 	qdio_stop_irq(CARD_DDEV(card));
 
-	if (IS_IQD(card)) {
-		struct qeth_qdio_out_q *queue;
-		unsigned int i;
-
-		/* Quiesce the NAPI instances: */
-		qeth_for_each_output_queue(card, queue, i)
-			napi_disable(&queue->napi);
+	/* Quiesce the NAPI instances: */
+	qeth_for_each_output_queue(card, queue, i)
+		napi_disable(&queue->napi);
 
-		/* Stop .ndo_start_xmit, might still access queue->napi. */
-		netif_tx_disable(dev);
+	/* Stop .ndo_start_xmit, might still access queue->napi. */
+	netif_tx_disable(dev);
 
-		qeth_for_each_output_queue(card, queue, i) {
-			del_timer_sync(&queue->timer);
-			/* Queues may get re-allocated, so remove the NAPIs. */
-			netif_napi_del(&queue->napi);
-		}
-	} else {
-		netif_tx_disable(dev);
+	qeth_for_each_output_queue(card, queue, i) {
+		del_timer_sync(&queue->timer);
+		/* Queues may get re-allocated, so remove the NAPIs. */
+		netif_napi_del(&queue->napi);
 	}
 
 	return 0;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next 3/9] s390/qeth: unify the tracking of active cmds on ccw device
  2021-06-11  7:33 [PATCH net-next 0/9] s390/qeth: updates 2021-06-11 Julian Wiedmann
  2021-06-11  7:33 ` [PATCH net-next 1/9] s390/qeth: count TX completion interrupts Julian Wiedmann
  2021-06-11  7:33 ` [PATCH net-next 2/9] s390/qeth: also use TX NAPI for non-IQD devices Julian Wiedmann
@ 2021-06-11  7:33 ` Julian Wiedmann
  2021-06-11  7:33 ` [PATCH net-next 4/9] s390/qeth: use ethtool_sprintf() Julian Wiedmann
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Julian Wiedmann @ 2021-06-11  7:33 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: linux-netdev, linux-s390, Heiko Carstens, Karsten Graul, Julian Wiedmann

We have one field to track _whether_ a cmd is active on a ccw device
('irq_pending'), and one to track _which_ cmd it is ('active_cmd').

Get rid of the irq_pending field, by testing active_cmd for NULL.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_core.h      | 14 +++++++-------
 drivers/s390/net/qeth_core_main.c | 12 ++++--------
 2 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index 4d29801bcf41..ad0e86aa99b2 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -614,7 +614,6 @@ struct qeth_channel {
 	struct ccw_device *ccwdev;
 	struct qeth_cmd_buffer *active_cmd;
 	enum qeth_channel_states state;
-	atomic_t irq_pending;
 };
 
 struct qeth_reply {
@@ -664,11 +663,6 @@ static inline struct ccw1 *__ccw_from_cmd(struct qeth_cmd_buffer *iob)
 	return (struct ccw1 *)(iob->data + ALIGN(iob->length, 8));
 }
 
-static inline bool qeth_trylock_channel(struct qeth_channel *channel)
-{
-	return atomic_cmpxchg(&channel->irq_pending, 0, 1) == 0;
-}
-
 /**
  *  OSA card related definitions
  */
@@ -896,10 +890,16 @@ static inline bool qeth_use_tx_irqs(struct qeth_card *card)
 static inline void qeth_unlock_channel(struct qeth_card *card,
 				       struct qeth_channel *channel)
 {
-	atomic_set(&channel->irq_pending, 0);
+	xchg(&channel->active_cmd, NULL);
 	wake_up(&card->wait_q);
 }
 
+static inline bool qeth_trylock_channel(struct qeth_channel *channel,
+					struct qeth_cmd_buffer *cmd)
+{
+	return cmpxchg(&channel->active_cmd, NULL, cmd) == NULL;
+}
+
 struct qeth_trap_id {
 	__u16 lparnr;
 	char vmname[8];
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index f22f223a4a6c..83d540f8b527 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -1268,7 +1268,6 @@ static void qeth_irq(struct ccw_device *cdev, unsigned long intparm,
 		iob = (struct qeth_cmd_buffer *) (addr_t)intparm;
 	}
 
-	channel->active_cmd = NULL;
 	qeth_unlock_channel(card, channel);
 
 	rc = qeth_check_irb_error(card, cdev, irb);
@@ -1715,11 +1714,10 @@ static int qeth_stop_channel(struct qeth_channel *channel)
 	rc = ccw_device_set_offline(cdev);
 
 	spin_lock_irq(get_ccwdev_lock(cdev));
-	if (channel->active_cmd) {
+	if (channel->active_cmd)
 		dev_err(&cdev->dev, "Stopped channel while cmd %px was still active\n",
 			channel->active_cmd);
-		channel->active_cmd = NULL;
-	}
+
 	cdev->handler = NULL;
 	spin_unlock_irq(get_ccwdev_lock(cdev));
 
@@ -1732,7 +1730,7 @@ static int qeth_start_channel(struct qeth_channel *channel)
 	int rc;
 
 	channel->state = CH_STATE_DOWN;
-	atomic_set(&channel->irq_pending, 0);
+	xchg(&channel->active_cmd, NULL);
 
 	spin_lock_irq(get_ccwdev_lock(cdev));
 	cdev->handler = qeth_irq;
@@ -2039,7 +2037,7 @@ static int qeth_send_control_data(struct qeth_card *card,
 	reply->param = reply_param;
 
 	timeout = wait_event_interruptible_timeout(card->wait_q,
-						   qeth_trylock_channel(channel),
+						   qeth_trylock_channel(channel, iob),
 						   timeout);
 	if (timeout <= 0) {
 		qeth_put_cmd(iob);
@@ -2059,8 +2057,6 @@ static int qeth_send_control_data(struct qeth_card *card,
 	spin_lock_irq(get_ccwdev_lock(channel->ccwdev));
 	rc = ccw_device_start_timeout(channel->ccwdev, __ccw_from_cmd(iob),
 				      (addr_t) iob, 0, 0, timeout);
-	if (!rc)
-		channel->active_cmd = iob;
 	spin_unlock_irq(get_ccwdev_lock(channel->ccwdev));
 	if (rc) {
 		QETH_DBF_MESSAGE(2, "qeth_send_control_data on device %x: ccw_device_start rc = %i\n",
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next 4/9] s390/qeth: use ethtool_sprintf()
  2021-06-11  7:33 [PATCH net-next 0/9] s390/qeth: updates 2021-06-11 Julian Wiedmann
                   ` (2 preceding siblings ...)
  2021-06-11  7:33 ` [PATCH net-next 3/9] s390/qeth: unify the tracking of active cmds on ccw device Julian Wiedmann
@ 2021-06-11  7:33 ` Julian Wiedmann
  2021-06-11  7:33 ` [PATCH net-next 5/9] s390/qeth: consolidate completion of pending TX buffers Julian Wiedmann
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Julian Wiedmann @ 2021-06-11  7:33 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: linux-netdev, linux-s390, Heiko Carstens, Karsten Graul, Julian Wiedmann

Use a recently introduced helper to fill our ethtool stats strings.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_ethtool.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/s390/net/qeth_ethtool.c b/drivers/s390/net/qeth_ethtool.c
index 190dac2065df..2c4cb300a8fc 100644
--- a/drivers/s390/net/qeth_ethtool.c
+++ b/drivers/s390/net/qeth_ethtool.c
@@ -80,10 +80,8 @@ static void qeth_add_stat_strings(u8 **data, const char *prefix,
 {
 	unsigned int i;
 
-	for (i = 0; i < size; i++) {
-		snprintf(*data, ETH_GSTRING_LEN, "%s%s", prefix, stats[i].name);
-		*data += ETH_GSTRING_LEN;
-	}
+	for (i = 0; i < size; i++)
+		ethtool_sprintf(data, "%s%s", prefix, stats[i].name);
 }
 
 static int qeth_get_sset_count(struct net_device *dev, int stringset)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next 5/9] s390/qeth: consolidate completion of pending TX buffers
  2021-06-11  7:33 [PATCH net-next 0/9] s390/qeth: updates 2021-06-11 Julian Wiedmann
                   ` (3 preceding siblings ...)
  2021-06-11  7:33 ` [PATCH net-next 4/9] s390/qeth: use ethtool_sprintf() Julian Wiedmann
@ 2021-06-11  7:33 ` Julian Wiedmann
  2021-06-11  7:33 ` [PATCH net-next 6/9] s390/qeth: remove QAOB's pointer to its TX buffer Julian Wiedmann
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Julian Wiedmann @ 2021-06-11  7:33 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: linux-netdev, linux-s390, Heiko Carstens, Karsten Graul, Julian Wiedmann

With commit 396c100472dd ("s390/qdio: let driver manage the QAOB")
a pending TX buffer now has access to its associated QAOB during
TX completion processing. We can thus reduce the amount of work & state
propagation that needs to be done by qeth_qdio_handle_aob().

Move all this logic into the respective TX completion paths. Doing so
even allows us to determine more precise TX_NOTIFY_* values via
qeth_compute_cq_notification(aob->aorc, ...).

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_core.h      |  3 +-
 drivers/s390/net/qeth_core_main.c | 73 ++++++++++++-------------------
 2 files changed, 29 insertions(+), 47 deletions(-)

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index ad0e86aa99b2..5de5b419a761 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -422,8 +422,7 @@ enum qeth_qdio_out_buffer_state {
 	/* Finished by the TX completion code: */
 	QETH_QDIO_BUF_NEED_QAOB,
 	/* Received QAOB notification on CQ: */
-	QETH_QDIO_BUF_QAOB_OK,
-	QETH_QDIO_BUF_QAOB_ERROR,
+	QETH_QDIO_BUF_QAOB_DONE,
 };
 
 struct qeth_qdio_out_buffer {
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 83d540f8b527..99e3b0b75cc3 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -70,9 +70,6 @@ static void qeth_issue_next_read_cb(struct qeth_card *card,
 				    unsigned int data_length);
 static int qeth_qdio_establish(struct qeth_card *);
 static void qeth_free_qdio_queues(struct qeth_card *card);
-static void qeth_notify_skbs(struct qeth_qdio_out_q *queue,
-		struct qeth_qdio_out_buffer *buf,
-		enum iucv_tx_notify notification);
 
 static void qeth_close_dev_handler(struct work_struct *work)
 {
@@ -437,12 +434,9 @@ static enum iucv_tx_notify qeth_compute_cq_notification(int sbalf15,
 static void qeth_qdio_handle_aob(struct qeth_card *card,
 				 unsigned long phys_aob_addr)
 {
-	enum qeth_qdio_out_buffer_state new_state = QETH_QDIO_BUF_QAOB_OK;
 	struct qaob *aob;
 	struct qeth_qdio_out_buffer *buffer;
-	enum iucv_tx_notify notification;
 	struct qeth_qdio_out_q *queue;
-	unsigned int i;
 
 	aob = (struct qaob *) phys_to_virt(phys_aob_addr);
 	QETH_CARD_TEXT(card, 5, "haob");
@@ -450,12 +444,10 @@ static void qeth_qdio_handle_aob(struct qeth_card *card,
 	buffer = (struct qeth_qdio_out_buffer *) aob->user1;
 	QETH_CARD_TEXT_(card, 5, "%lx", aob->user1);
 
-	if (aob->aorc) {
+	if (aob->aorc)
 		QETH_CARD_TEXT_(card, 2, "aorc%02X", aob->aorc);
-		new_state = QETH_QDIO_BUF_QAOB_ERROR;
-	}
 
-	switch (atomic_xchg(&buffer->state, new_state)) {
+	switch (atomic_xchg(&buffer->state, QETH_QDIO_BUF_QAOB_DONE)) {
 	case QETH_QDIO_BUF_PRIMED:
 		/* Faster than TX completion code, let it handle the async
 		 * completion for us. It will also recycle the QAOB.
@@ -468,21 +460,6 @@ static void qeth_qdio_handle_aob(struct qeth_card *card,
 		break;
 	case QETH_QDIO_BUF_NEED_QAOB:
 		/* TX completion code is already finished. */
-		notification = qeth_compute_cq_notification(aob->aorc, 1);
-		qeth_notify_skbs(buffer->q, buffer, notification);
-
-		/* Free dangling allocations. The attached skbs are handled by
-		 * qeth_tx_complete_pending_bufs(), and so is the QAOB.
-		 */
-		for (i = 0;
-		     i < aob->sb_count && i < QETH_MAX_BUFFER_ELEMENTS(card);
-		     i++) {
-			void *data = phys_to_virt(aob->sba[i]);
-
-			if (data && buffer->is_header[i])
-				kmem_cache_free(qeth_core_header_cache, data);
-			buffer->is_header[i] = 0;
-		}
 
 		queue = buffer->q;
 		atomic_set(&buffer->state, QETH_QDIO_BUF_EMPTY);
@@ -1435,15 +1412,29 @@ static void qeth_tx_complete_pending_bufs(struct qeth_card *card,
 	struct qeth_qdio_out_buffer *buf, *tmp;
 
 	list_for_each_entry_safe(buf, tmp, &queue->pending_bufs, list_entry) {
+		struct qaob *aob = buf->aob;
+		enum iucv_tx_notify notify;
+		unsigned int i;
+
 		if (drain || atomic_read(&buf->state) == QETH_QDIO_BUF_EMPTY) {
 			QETH_CARD_TEXT(card, 5, "fp");
 			QETH_CARD_TEXT_(card, 5, "%lx", (long) buf);
 
-			if (drain)
-				qeth_notify_skbs(queue, buf,
-						 TX_NOTIFY_GENERALERROR);
+			notify = drain ? TX_NOTIFY_GENERALERROR :
+					 qeth_compute_cq_notification(aob->aorc, 1);
+			qeth_notify_skbs(queue, buf, notify);
 			qeth_tx_complete_buf(buf, drain, budget);
 
+			for (i = 0;
+			     i < aob->sb_count && i < queue->max_elements;
+			     i++) {
+				void *data = phys_to_virt(aob->sba[i]);
+
+				if (data && buf->is_header[i])
+					kmem_cache_free(qeth_core_header_cache,
+							data);
+			}
+
 			list_del(&buf->list_entry);
 			qeth_free_out_buf(buf);
 		}
@@ -6048,6 +6039,7 @@ static void qeth_iqd_tx_complete(struct qeth_qdio_out_q *queue,
 
 	if (qdio_error == QDIO_ERROR_SLSB_PENDING) {
 		struct qaob *aob = buffer->aob;
+		enum iucv_tx_notify notify;
 
 		if (!aob) {
 			netdev_WARN_ONCE(card->dev,
@@ -6084,30 +6076,21 @@ static void qeth_iqd_tx_complete(struct qeth_qdio_out_q *queue,
 					 &queue->pending_bufs);
 				/* Skip clearing the buffer: */
 				return;
-			case QETH_QDIO_BUF_QAOB_OK:
-				qeth_notify_skbs(queue, buffer,
-						 TX_NOTIFY_DELAYED_OK);
-				error = false;
-				break;
-			case QETH_QDIO_BUF_QAOB_ERROR:
-				qeth_notify_skbs(queue, buffer,
-						 TX_NOTIFY_DELAYED_GENERALERROR);
-				error = true;
+			case QETH_QDIO_BUF_QAOB_DONE:
+				notify = qeth_compute_cq_notification(aob->aorc, 1);
+				qeth_notify_skbs(queue, buffer, notify);
+				error = !!aob->aorc;
 				break;
 			default:
 				WARN_ON_ONCE(1);
 			}
 
 			break;
-		case QETH_QDIO_BUF_QAOB_OK:
-			/* qeth_qdio_handle_aob() already received a QAOB: */
-			qeth_notify_skbs(queue, buffer, TX_NOTIFY_OK);
-			error = false;
-			break;
-		case QETH_QDIO_BUF_QAOB_ERROR:
+		case QETH_QDIO_BUF_QAOB_DONE:
 			/* qeth_qdio_handle_aob() already received a QAOB: */
-			qeth_notify_skbs(queue, buffer, TX_NOTIFY_GENERALERROR);
-			error = true;
+			notify = qeth_compute_cq_notification(aob->aorc, 0);
+			qeth_notify_skbs(queue, buffer, notify);
+			error = !!aob->aorc;
 			break;
 		default:
 			WARN_ON_ONCE(1);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next 6/9] s390/qeth: remove QAOB's pointer to its TX buffer
  2021-06-11  7:33 [PATCH net-next 0/9] s390/qeth: updates 2021-06-11 Julian Wiedmann
                   ` (4 preceding siblings ...)
  2021-06-11  7:33 ` [PATCH net-next 5/9] s390/qeth: consolidate completion of pending TX buffers Julian Wiedmann
@ 2021-06-11  7:33 ` Julian Wiedmann
  2021-06-11  7:33 ` [PATCH net-next 7/9] s390/qeth: remove TX buffer's pointer to its queue Julian Wiedmann
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Julian Wiedmann @ 2021-06-11  7:33 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: linux-netdev, linux-s390, Heiko Carstens, Karsten Graul, Julian Wiedmann

Maintaining a pointer inside the aob's user-definable area is fragile
and unnecessary. At this stage we only need it to overload the buffer's
state field, and to access the buffer's TX queue.

The first part is easily solved by tracking the aob's state within the
aob itself. This also feels much cleaner and self-contained.
For enabling the access to the associated TX queue, we can store the
queue's index in the aob.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 arch/s390/include/asm/qdio.h      |   4 +-
 drivers/s390/net/qeth_core.h      |  17 +++--
 drivers/s390/net/qeth_core_main.c | 118 ++++++++++--------------------
 3 files changed, 49 insertions(+), 90 deletions(-)

diff --git a/arch/s390/include/asm/qdio.h b/arch/s390/include/asm/qdio.h
index 8fc52679543d..cb4f73c7228d 100644
--- a/arch/s390/include/asm/qdio.h
+++ b/arch/s390/include/asm/qdio.h
@@ -137,7 +137,6 @@ struct slibe {
  * @user0: user defineable value
  * @res4: reserved paramater
  * @user1: user defineable value
- * @user2: user defineable value
  */
 struct qaob {
 	u64 res0[6];
@@ -152,8 +151,7 @@ struct qaob {
 	u16 dcount[QDIO_MAX_ELEMENTS_PER_BUFFER];
 	u64 user0;
 	u64 res4[2];
-	u64 user1;
-	u64 user2;
+	u8 user1[16];
 } __attribute__ ((packed, aligned(256)));
 
 /**
diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index 5de5b419a761..457224b7b97f 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -417,12 +417,17 @@ enum qeth_qdio_out_buffer_state {
 	QETH_QDIO_BUF_EMPTY,
 	/* Filled by driver; owned by hardware in order to be sent. */
 	QETH_QDIO_BUF_PRIMED,
-	/* Discovered by the TX completion code: */
-	QETH_QDIO_BUF_PENDING,
-	/* Finished by the TX completion code: */
-	QETH_QDIO_BUF_NEED_QAOB,
-	/* Received QAOB notification on CQ: */
-	QETH_QDIO_BUF_QAOB_DONE,
+};
+
+enum qeth_qaob_state {
+	QETH_QAOB_ISSUED,
+	QETH_QAOB_PENDING,
+	QETH_QAOB_DONE,
+};
+
+struct qeth_qaob_priv1 {
+	unsigned int state;
+	u8 queue_no;
 };
 
 struct qeth_qdio_out_buffer {
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 99e3b0b75cc3..5ddb2939d4fc 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -431,45 +431,6 @@ static enum iucv_tx_notify qeth_compute_cq_notification(int sbalf15,
 	return n;
 }
 
-static void qeth_qdio_handle_aob(struct qeth_card *card,
-				 unsigned long phys_aob_addr)
-{
-	struct qaob *aob;
-	struct qeth_qdio_out_buffer *buffer;
-	struct qeth_qdio_out_q *queue;
-
-	aob = (struct qaob *) phys_to_virt(phys_aob_addr);
-	QETH_CARD_TEXT(card, 5, "haob");
-	QETH_CARD_TEXT_(card, 5, "%lx", phys_aob_addr);
-	buffer = (struct qeth_qdio_out_buffer *) aob->user1;
-	QETH_CARD_TEXT_(card, 5, "%lx", aob->user1);
-
-	if (aob->aorc)
-		QETH_CARD_TEXT_(card, 2, "aorc%02X", aob->aorc);
-
-	switch (atomic_xchg(&buffer->state, QETH_QDIO_BUF_QAOB_DONE)) {
-	case QETH_QDIO_BUF_PRIMED:
-		/* Faster than TX completion code, let it handle the async
-		 * completion for us. It will also recycle the QAOB.
-		 */
-		break;
-	case QETH_QDIO_BUF_PENDING:
-		/* TX completion code is active and will handle the async
-		 * completion for us. It will also recycle the QAOB.
-		 */
-		break;
-	case QETH_QDIO_BUF_NEED_QAOB:
-		/* TX completion code is already finished. */
-
-		queue = buffer->q;
-		atomic_set(&buffer->state, QETH_QDIO_BUF_EMPTY);
-		napi_schedule(&queue->napi);
-		break;
-	default:
-		WARN_ON_ONCE(1);
-	}
-}
-
 static void qeth_setup_ccw(struct ccw1 *ccw, u8 cmd_code, u8 flags, u32 len,
 			   void *data)
 {
@@ -1412,11 +1373,13 @@ static void qeth_tx_complete_pending_bufs(struct qeth_card *card,
 	struct qeth_qdio_out_buffer *buf, *tmp;
 
 	list_for_each_entry_safe(buf, tmp, &queue->pending_bufs, list_entry) {
+		struct qeth_qaob_priv1 *priv;
 		struct qaob *aob = buf->aob;
 		enum iucv_tx_notify notify;
 		unsigned int i;
 
-		if (drain || atomic_read(&buf->state) == QETH_QDIO_BUF_EMPTY) {
+		priv = (struct qeth_qaob_priv1 *)&aob->user1;
+		if (drain || READ_ONCE(priv->state) == QETH_QAOB_DONE) {
 			QETH_CARD_TEXT(card, 5, "fp");
 			QETH_CARD_TEXT_(card, 5, "%lx", (long) buf);
 
@@ -3625,8 +3588,12 @@ static void qeth_flush_buffers(struct qeth_qdio_out_q *queue, int index,
 			if (!buf->aob)
 				buf->aob = qdio_allocate_aob();
 			if (buf->aob) {
+				struct qeth_qaob_priv1 *priv;
+
 				aob = buf->aob;
-				aob->user1 = (u64) buf;
+				priv = (struct qeth_qaob_priv1 *)&aob->user1;
+				priv->state = QETH_QAOB_ISSUED;
+				priv->queue_no = queue->queue_no;
 			}
 		}
 	} else {
@@ -3765,6 +3732,18 @@ int qeth_configure_cq(struct qeth_card *card, enum qeth_cq cq)
 }
 EXPORT_SYMBOL_GPL(qeth_configure_cq);
 
+static void qeth_qdio_handle_aob(struct qeth_card *card, struct qaob *aob)
+{
+	struct qeth_qaob_priv1 *priv = (struct qeth_qaob_priv1 *)&aob->user1;
+	unsigned int queue_no = priv->queue_no;
+
+	BUILD_BUG_ON(sizeof(*priv) > ARRAY_SIZE(aob->user1));
+
+	if (xchg(&priv->state, QETH_QAOB_DONE) == QETH_QAOB_PENDING &&
+	    queue_no < card->qdio.no_out_queues)
+		napi_schedule(&card->qdio.out_qs[queue_no]->napi);
+}
+
 static void qeth_qdio_cq_handler(struct qeth_card *card, unsigned int qdio_err,
 				 unsigned int queue, int first_element,
 				 int count)
@@ -3791,7 +3770,7 @@ static void qeth_qdio_cq_handler(struct qeth_card *card, unsigned int qdio_err,
 		       buffer->element[e].addr) {
 			unsigned long phys_aob_addr = buffer->element[e].addr;
 
-			qeth_qdio_handle_aob(card, phys_aob_addr);
+			qeth_qdio_handle_aob(card, phys_to_virt(phys_aob_addr));
 			++e;
 		}
 		qeth_scrub_qdio_buffer(buffer, QDIO_MAX_ELEMENTS_PER_BUFFER);
@@ -6039,6 +6018,7 @@ static void qeth_iqd_tx_complete(struct qeth_qdio_out_q *queue,
 
 	if (qdio_error == QDIO_ERROR_SLSB_PENDING) {
 		struct qaob *aob = buffer->aob;
+		struct qeth_qaob_priv1 *priv;
 		enum iucv_tx_notify notify;
 
 		if (!aob) {
@@ -6051,51 +6031,27 @@ static void qeth_iqd_tx_complete(struct qeth_qdio_out_q *queue,
 
 		QETH_CARD_TEXT_(card, 5, "pel%u", bidx);
 
-		switch (atomic_cmpxchg(&buffer->state,
-				       QETH_QDIO_BUF_PRIMED,
-				       QETH_QDIO_BUF_PENDING)) {
-		case QETH_QDIO_BUF_PRIMED:
-			/* We have initial ownership, no QAOB (yet): */
+		priv = (struct qeth_qaob_priv1 *)&aob->user1;
+		/* QAOB hasn't completed yet: */
+		if (xchg(&priv->state, QETH_QAOB_PENDING) != QETH_QAOB_DONE) {
 			qeth_notify_skbs(queue, buffer, TX_NOTIFY_PENDING);
 
-			/* Handle race with qeth_qdio_handle_aob(): */
-			switch (atomic_xchg(&buffer->state,
-					    QETH_QDIO_BUF_NEED_QAOB)) {
-			case QETH_QDIO_BUF_PENDING:
-				/* No concurrent QAOB notification. */
-
-				/* Prepare the queue slot for immediate re-use: */
-				qeth_scrub_qdio_buffer(buffer->buffer, queue->max_elements);
-				if (qeth_alloc_out_buf(queue, bidx,
-						       GFP_ATOMIC)) {
-					QETH_CARD_TEXT(card, 2, "outofbuf");
-					qeth_schedule_recovery(card);
-				}
-
-				list_add(&buffer->list_entry,
-					 &queue->pending_bufs);
-				/* Skip clearing the buffer: */
-				return;
-			case QETH_QDIO_BUF_QAOB_DONE:
-				notify = qeth_compute_cq_notification(aob->aorc, 1);
-				qeth_notify_skbs(queue, buffer, notify);
-				error = !!aob->aorc;
-				break;
-			default:
-				WARN_ON_ONCE(1);
+			/* Prepare the queue slot for immediate re-use: */
+			qeth_scrub_qdio_buffer(buffer->buffer, queue->max_elements);
+			if (qeth_alloc_out_buf(queue, bidx, GFP_ATOMIC)) {
+				QETH_CARD_TEXT(card, 2, "outofbuf");
+				qeth_schedule_recovery(card);
 			}
 
-			break;
-		case QETH_QDIO_BUF_QAOB_DONE:
-			/* qeth_qdio_handle_aob() already received a QAOB: */
-			notify = qeth_compute_cq_notification(aob->aorc, 0);
-			qeth_notify_skbs(queue, buffer, notify);
-			error = !!aob->aorc;
-			break;
-		default:
-			WARN_ON_ONCE(1);
+			list_add(&buffer->list_entry, &queue->pending_bufs);
+			/* Skip clearing the buffer: */
+			return;
 		}
 
+		/* QAOB already completed: */
+		notify = qeth_compute_cq_notification(aob->aorc, 0);
+		qeth_notify_skbs(queue, buffer, notify);
+		error = !!aob->aorc;
 		memset(aob, 0, sizeof(*aob));
 	} else if (card->options.cq == QETH_CQ_ENABLED) {
 		qeth_notify_skbs(queue, buffer,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next 7/9] s390/qeth: remove TX buffer's pointer to its queue
  2021-06-11  7:33 [PATCH net-next 0/9] s390/qeth: updates 2021-06-11 Julian Wiedmann
                   ` (5 preceding siblings ...)
  2021-06-11  7:33 ` [PATCH net-next 6/9] s390/qeth: remove QAOB's pointer to its TX buffer Julian Wiedmann
@ 2021-06-11  7:33 ` Julian Wiedmann
  2021-06-11  7:33 ` [PATCH net-next 8/9] s390/qeth: shrink TX buffer struct Julian Wiedmann
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Julian Wiedmann @ 2021-06-11  7:33 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: linux-netdev, linux-s390, Heiko Carstens, Karsten Graul, Julian Wiedmann

qeth_tx_complete_buf() is the only remaining user of buf->q, and the
callers can easily provide this as a parameter instead.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_core.h      | 1 -
 drivers/s390/net/qeth_core_main.c | 9 ++++-----
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index 457224b7b97f..ff1064f871e5 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -439,7 +439,6 @@ struct qeth_qdio_out_buffer {
 	struct sk_buff_head skb_list;
 	int is_header[QDIO_MAX_ELEMENTS_PER_BUFFER];
 
-	struct qeth_qdio_out_q *q;
 	struct list_head list_entry;
 	struct qaob *aob;
 };
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 5ddb2939d4fc..0ad175d54c13 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -1290,10 +1290,10 @@ static void qeth_notify_skbs(struct qeth_qdio_out_q *q,
 	}
 }
 
-static void qeth_tx_complete_buf(struct qeth_qdio_out_buffer *buf, bool error,
+static void qeth_tx_complete_buf(struct qeth_qdio_out_q *queue,
+				 struct qeth_qdio_out_buffer *buf, bool error,
 				 int budget)
 {
-	struct qeth_qdio_out_q *queue = buf->q;
 	struct sk_buff *skb;
 
 	/* Empty buffer? */
@@ -1342,7 +1342,7 @@ static void qeth_clear_output_buffer(struct qeth_qdio_out_q *queue,
 		QETH_TXQ_STAT_INC(queue, completion_irq);
 	}
 
-	qeth_tx_complete_buf(buf, error, budget);
+	qeth_tx_complete_buf(queue, buf, error, budget);
 
 	for (i = 0; i < queue->max_elements; ++i) {
 		void *data = phys_to_virt(buf->buffer->element[i].addr);
@@ -1386,7 +1386,7 @@ static void qeth_tx_complete_pending_bufs(struct qeth_card *card,
 			notify = drain ? TX_NOTIFY_GENERALERROR :
 					 qeth_compute_cq_notification(aob->aorc, 1);
 			qeth_notify_skbs(queue, buf, notify);
-			qeth_tx_complete_buf(buf, drain, budget);
+			qeth_tx_complete_buf(queue, buf, drain, budget);
 
 			for (i = 0;
 			     i < aob->sb_count && i < queue->max_elements;
@@ -2530,7 +2530,6 @@ static int qeth_alloc_out_buf(struct qeth_qdio_out_q *q, unsigned int bidx,
 	newbuf->buffer = q->qdio_bufs[bidx];
 	skb_queue_head_init(&newbuf->skb_list);
 	lockdep_set_class(&newbuf->skb_list.lock, &qdio_out_skb_queue_key);
-	newbuf->q = q;
 	atomic_set(&newbuf->state, QETH_QDIO_BUF_EMPTY);
 	q->bufs[bidx] = newbuf;
 	return 0;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next 8/9] s390/qeth: shrink TX buffer struct
  2021-06-11  7:33 [PATCH net-next 0/9] s390/qeth: updates 2021-06-11 Julian Wiedmann
                   ` (6 preceding siblings ...)
  2021-06-11  7:33 ` [PATCH net-next 7/9] s390/qeth: remove TX buffer's pointer to its queue Julian Wiedmann
@ 2021-06-11  7:33 ` Julian Wiedmann
  2021-06-11  7:33 ` [PATCH net-next 9/9] s390/qeth: Consider dependency on SWITCHDEV module Julian Wiedmann
  2021-06-11 20:00 ` [PATCH net-next 0/9] s390/qeth: updates 2021-06-11 patchwork-bot+netdevbpf
  9 siblings, 0 replies; 11+ messages in thread
From: Julian Wiedmann @ 2021-06-11  7:33 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: linux-netdev, linux-s390, Heiko Carstens, Karsten Graul, Julian Wiedmann

Convert the large boolean array into a bitmap, this substantially
reduces the struct's size. While at it also clarify the naming.

Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_core.h      | 2 +-
 drivers/s390/net/qeth_core_main.c | 7 +++----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index ff1064f871e5..f4d554ea0c93 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -437,7 +437,7 @@ struct qeth_qdio_out_buffer {
 	unsigned int frames;
 	unsigned int bytes;
 	struct sk_buff_head skb_list;
-	int is_header[QDIO_MAX_ELEMENTS_PER_BUFFER];
+	DECLARE_BITMAP(from_kmem_cache, QDIO_MAX_ELEMENTS_PER_BUFFER);
 
 	struct list_head list_entry;
 	struct qaob *aob;
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index 0ad175d54c13..62f88ccbd03f 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -1347,9 +1347,8 @@ static void qeth_clear_output_buffer(struct qeth_qdio_out_q *queue,
 	for (i = 0; i < queue->max_elements; ++i) {
 		void *data = phys_to_virt(buf->buffer->element[i].addr);
 
-		if (data && buf->is_header[i])
+		if (__test_and_clear_bit(i, buf->from_kmem_cache) && data)
 			kmem_cache_free(qeth_core_header_cache, data);
-		buf->is_header[i] = 0;
 	}
 
 	qeth_scrub_qdio_buffer(buf->buffer, queue->max_elements);
@@ -1393,7 +1392,7 @@ static void qeth_tx_complete_pending_bufs(struct qeth_card *card,
 			     i++) {
 				void *data = phys_to_virt(aob->sba[i]);
 
-				if (data && buf->is_header[i])
+				if (test_bit(i, buf->from_kmem_cache) && data)
 					kmem_cache_free(qeth_core_header_cache,
 							data);
 			}
@@ -4053,7 +4052,7 @@ static unsigned int qeth_fill_buffer(struct qeth_qdio_out_buffer *buf,
 
 		/* HW header is allocated from cache: */
 		if ((void *)hdr != skb->data)
-			buf->is_header[element] = 1;
+			__set_bit(element, buf->from_kmem_cache);
 		/* HW header was pushed and is contiguous with linear part: */
 		else if (length > 0 && !PAGE_ALIGNED(data) &&
 			 (data == (char *)hdr + hd_len))
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next 9/9] s390/qeth: Consider dependency on SWITCHDEV module
  2021-06-11  7:33 [PATCH net-next 0/9] s390/qeth: updates 2021-06-11 Julian Wiedmann
                   ` (7 preceding siblings ...)
  2021-06-11  7:33 ` [PATCH net-next 8/9] s390/qeth: shrink TX buffer struct Julian Wiedmann
@ 2021-06-11  7:33 ` Julian Wiedmann
  2021-06-11 20:00 ` [PATCH net-next 0/9] s390/qeth: updates 2021-06-11 patchwork-bot+netdevbpf
  9 siblings, 0 replies; 11+ messages in thread
From: Julian Wiedmann @ 2021-06-11  7:33 UTC (permalink / raw)
  To: David Miller, Jakub Kicinski
  Cc: linux-netdev, linux-s390, Heiko Carstens, Karsten Graul,
	Julian Wiedmann, Alexandra Winter

From: Alexandra Winter <wintera@linux.ibm.com>

Without the SWITCHDEV module, the bridgeport attribute LEARNING_SYNC
of the physical device (self) does not provide any functionality.
Instead of calling the no-op stub version of the switchdev functions,
fail the setting of the attribute with an appropriate message.

While at it, also add an error message for the 'not supported by HW'
case.

Signed-off-by: Alexandra Winter <wintera@linux.ibm.com>
Reviewed-by: Julian Wiedmann <jwi@linux.ibm.com>
Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
 drivers/s390/net/qeth_l2_main.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index ca44421a6d6e..2abf86c104d5 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -805,8 +805,6 @@ static int qeth_l2_bridge_setlink(struct net_device *dev, struct nlmsghdr *nlh,
 
 	if (!netif_device_present(dev))
 		return -ENODEV;
-	if (!(priv->brport_hw_features))
-		return -EOPNOTSUPP;
 
 	nlmsg_for_each_attr(attr, nlh, sizeof(struct ifinfomsg), rem1) {
 		if (nla_type(attr) == IFLA_PROTINFO) {
@@ -832,6 +830,16 @@ static int qeth_l2_bridge_setlink(struct net_device *dev, struct nlmsghdr *nlh,
 		return 0;
 	if (!bp_tb[IFLA_BRPORT_LEARNING_SYNC])
 		return -EINVAL;
+	if (!(priv->brport_hw_features & BR_LEARNING_SYNC)) {
+		NL_SET_ERR_MSG_ATTR(extack, bp_tb[IFLA_BRPORT_LEARNING_SYNC],
+				    "Operation not supported by HW");
+		return -EOPNOTSUPP;
+	}
+	if (!IS_ENABLED(CONFIG_NET_SWITCHDEV)) {
+		NL_SET_ERR_MSG_ATTR(extack, bp_tb[IFLA_BRPORT_LEARNING_SYNC],
+				    "Requires NET_SWITCHDEV");
+		return -EOPNOTSUPP;
+	}
 	enable = !!nla_get_u8(bp_tb[IFLA_BRPORT_LEARNING_SYNC]);
 
 	if (enable == !!(priv->brport_features & BR_LEARNING_SYNC))
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next 0/9] s390/qeth: updates 2021-06-11
  2021-06-11  7:33 [PATCH net-next 0/9] s390/qeth: updates 2021-06-11 Julian Wiedmann
                   ` (8 preceding siblings ...)
  2021-06-11  7:33 ` [PATCH net-next 9/9] s390/qeth: Consider dependency on SWITCHDEV module Julian Wiedmann
@ 2021-06-11 20:00 ` patchwork-bot+netdevbpf
  9 siblings, 0 replies; 11+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-06-11 20:00 UTC (permalink / raw)
  To: Julian Wiedmann; +Cc: davem, kuba, netdev, linux-s390, hca, kgraul

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Fri, 11 Jun 2021 09:33:32 +0200 you wrote:
> Hi Dave & Jakub,
> 
> please apply the following patch series for qeth to netdev's net-next tree.
> 
> This enables TX NAPI for those devices that didn't use it previously, so
> that we can eventually rip out the qdio layer's internal interrupt
> machinery.
> 
> [...]

Here is the summary with links:
  - [net-next,1/9] s390/qeth: count TX completion interrupts
    https://git.kernel.org/netdev/net-next/c/e872d0c1249b
  - [net-next,2/9] s390/qeth: also use TX NAPI for non-IQD devices
    https://git.kernel.org/netdev/net-next/c/7a4b92e8e0de
  - [net-next,3/9] s390/qeth: unify the tracking of active cmds on ccw device
    https://git.kernel.org/netdev/net-next/c/3518ae76f2bb
  - [net-next,4/9] s390/qeth: use ethtool_sprintf()
    https://git.kernel.org/netdev/net-next/c/c0a0186630fb
  - [net-next,5/9] s390/qeth: consolidate completion of pending TX buffers
    https://git.kernel.org/netdev/net-next/c/f875d880f049
  - [net-next,6/9] s390/qeth: remove QAOB's pointer to its TX buffer
    https://git.kernel.org/netdev/net-next/c/838e4cc80814
  - [net-next,7/9] s390/qeth: remove TX buffer's pointer to its queue
    https://git.kernel.org/netdev/net-next/c/6b7ec41e574a
  - [net-next,8/9] s390/qeth: shrink TX buffer struct
    https://git.kernel.org/netdev/net-next/c/bb7032ddc947
  - [net-next,9/9] s390/qeth: Consider dependency on SWITCHDEV module
    https://git.kernel.org/netdev/net-next/c/953fb4dc4f4a

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-06-11 20:00 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-11  7:33 [PATCH net-next 0/9] s390/qeth: updates 2021-06-11 Julian Wiedmann
2021-06-11  7:33 ` [PATCH net-next 1/9] s390/qeth: count TX completion interrupts Julian Wiedmann
2021-06-11  7:33 ` [PATCH net-next 2/9] s390/qeth: also use TX NAPI for non-IQD devices Julian Wiedmann
2021-06-11  7:33 ` [PATCH net-next 3/9] s390/qeth: unify the tracking of active cmds on ccw device Julian Wiedmann
2021-06-11  7:33 ` [PATCH net-next 4/9] s390/qeth: use ethtool_sprintf() Julian Wiedmann
2021-06-11  7:33 ` [PATCH net-next 5/9] s390/qeth: consolidate completion of pending TX buffers Julian Wiedmann
2021-06-11  7:33 ` [PATCH net-next 6/9] s390/qeth: remove QAOB's pointer to its TX buffer Julian Wiedmann
2021-06-11  7:33 ` [PATCH net-next 7/9] s390/qeth: remove TX buffer's pointer to its queue Julian Wiedmann
2021-06-11  7:33 ` [PATCH net-next 8/9] s390/qeth: shrink TX buffer struct Julian Wiedmann
2021-06-11  7:33 ` [PATCH net-next 9/9] s390/qeth: Consider dependency on SWITCHDEV module Julian Wiedmann
2021-06-11 20:00 ` [PATCH net-next 0/9] s390/qeth: updates 2021-06-11 patchwork-bot+netdevbpf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.