All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/2] sfc: simplify mtd partitions list handling
@ 2022-05-24  6:22 Íñigo Huguet
  2022-05-24  6:22 ` [PATCH net-next v2 1/2] " Íñigo Huguet
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Íñigo Huguet @ 2022-05-24  6:22 UTC (permalink / raw)
  To: ecree.xilinx, habetsm.xilinx
  Cc: davem, edumazet, kuba, pabeni, netdev, Íñigo Huguet

Simplify the mtd list handling to make more clear how it works and avoid
potential future problems.

Tested: module load and unload in a system with Siena and EF10 cards
installed, both modules sfc and sfc_siena.

v2:
* Dropped patch fixing memory leak, already fixed in net
* Apply changes also in new siena driver

Íñigo Huguet (2):
  sfc: simplify mtd partitions list handling
  sfc/siena: simplify mtd partitions list handling

 drivers/net/ethernet/sfc/ef10.c             | 12 ++++--
 drivers/net/ethernet/sfc/efx.h              |  4 +-
 drivers/net/ethernet/sfc/efx_common.c       |  3 --
 drivers/net/ethernet/sfc/mtd.c              | 42 ++++++++-------------
 drivers/net/ethernet/sfc/net_driver.h       |  9 +++--
 drivers/net/ethernet/sfc/siena/efx.h        |  4 +-
 drivers/net/ethernet/sfc/siena/efx_common.c |  3 --
 drivers/net/ethernet/sfc/siena/mtd.c        | 42 ++++++++-------------
 drivers/net/ethernet/sfc/siena/net_driver.h |  9 +++--
 drivers/net/ethernet/sfc/siena/siena.c      | 12 ++++--
 10 files changed, 66 insertions(+), 74 deletions(-)

-- 
2.34.1


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

* [PATCH net-next v2 1/2] sfc: simplify mtd partitions list handling
  2022-05-24  6:22 [PATCH net-next v2 0/2] sfc: simplify mtd partitions list handling Íñigo Huguet
@ 2022-05-24  6:22 ` Íñigo Huguet
  2022-05-27  6:17   ` Martin Habets
  2022-05-24  6:22 ` [PATCH net-next v2 2/2] sfc/siena: " Íñigo Huguet
  2022-05-24 10:17 ` [PATCH net-next v2 0/2] sfc: " Paolo Abeni
  2 siblings, 1 reply; 6+ messages in thread
From: Íñigo Huguet @ 2022-05-24  6:22 UTC (permalink / raw)
  To: ecree.xilinx, habetsm.xilinx
  Cc: davem, edumazet, kuba, pabeni, netdev, Íñigo Huguet

efx_mtd_partitions are embedded inside efx_mcdi_mtd_partition structs.
They contain a list entry that is appended to efx->mtd_list, which is
traversed to perform add/remove/rename/etc operations over all
efx_mtd_partitions.

However, almost all operations done on a efx_mtd_partition asume that it
is actually embedded inside an efx_mcdi_mtd_partition, and the
deallocation asume that the first member of the list is located at the
beginning of the allocated memory.

Given all that asumptions, the possibility of having an
efx_mtd_partition not embedded in an efx_mcdi_efx_partition doesn't
exist. Neither it does the possibility of being in a memory position
other the one allocated for the efx_mcdi_mtd_partition array. Also, they
never need to be reordered.

Given all that, it is better to get rid of the list and use directly the
efx_mcdi_mtd_partition array. This shows more clearly how they lay
in memory, list traversal is more obvious and it save a small amount
of memory on the list nodes.

Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
---
 drivers/net/ethernet/sfc/ef10.c       | 12 ++++++--
 drivers/net/ethernet/sfc/efx.h        |  4 +--
 drivers/net/ethernet/sfc/efx_common.c |  3 --
 drivers/net/ethernet/sfc/mtd.c        | 42 ++++++++++-----------------
 drivers/net/ethernet/sfc/net_driver.h |  9 ++++--
 5 files changed, 33 insertions(+), 37 deletions(-)

diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c
index 186cb28c03bd..89f8c1e63ec0 100644
--- a/drivers/net/ethernet/sfc/ef10.c
+++ b/drivers/net/ethernet/sfc/ef10.c
@@ -3584,10 +3584,16 @@ static int efx_ef10_mtd_probe(struct efx_nic *efx)
 		return 0;
 	}
 
-	rc = efx_mtd_add(efx, &parts[0].common, n_parts, sizeof(*parts));
-fail:
+	rc = efx_mtd_add(efx, parts, n_parts);
 	if (rc)
-		kfree(parts);
+		goto fail;
+	efx->mcdi_mtd_parts = parts;
+	efx->n_mcdi_mtd_parts = n_parts;
+
+	return 0;
+
+fail:
+	kfree(parts);
 	return rc;
 }
 
diff --git a/drivers/net/ethernet/sfc/efx.h b/drivers/net/ethernet/sfc/efx.h
index c05a83da9e44..2ab9ba691b0d 100644
--- a/drivers/net/ethernet/sfc/efx.h
+++ b/drivers/net/ethernet/sfc/efx.h
@@ -181,8 +181,8 @@ void efx_update_sw_stats(struct efx_nic *efx, u64 *stats);
 
 /* MTD */
 #ifdef CONFIG_SFC_MTD
-int efx_mtd_add(struct efx_nic *efx, struct efx_mtd_partition *parts,
-		size_t n_parts, size_t sizeof_part);
+int efx_mtd_add(struct efx_nic *efx, struct efx_mcdi_mtd_partition *parts,
+		size_t n_parts);
 static inline int efx_mtd_probe(struct efx_nic *efx)
 {
 	return efx->type->mtd_probe(efx);
diff --git a/drivers/net/ethernet/sfc/efx_common.c b/drivers/net/ethernet/sfc/efx_common.c
index f6577e74d6e6..8802790403e9 100644
--- a/drivers/net/ethernet/sfc/efx_common.c
+++ b/drivers/net/ethernet/sfc/efx_common.c
@@ -987,9 +987,6 @@ int efx_init_struct(struct efx_nic *efx,
 	INIT_LIST_HEAD(&efx->node);
 	INIT_LIST_HEAD(&efx->secondary_list);
 	spin_lock_init(&efx->biu_lock);
-#ifdef CONFIG_SFC_MTD
-	INIT_LIST_HEAD(&efx->mtd_list);
-#endif
 	INIT_WORK(&efx->reset_work, efx_reset_work);
 	INIT_DELAYED_WORK(&efx->monitor_work, efx_monitor);
 	efx_selftest_async_init(efx);
diff --git a/drivers/net/ethernet/sfc/mtd.c b/drivers/net/ethernet/sfc/mtd.c
index 273c08e5455f..4d06e8a9a729 100644
--- a/drivers/net/ethernet/sfc/mtd.c
+++ b/drivers/net/ethernet/sfc/mtd.c
@@ -12,6 +12,7 @@
 
 #include "net_driver.h"
 #include "efx.h"
+#include "mcdi.h"
 
 #define to_efx_mtd_partition(mtd)				\
 	container_of(mtd, struct efx_mtd_partition, mtd)
@@ -48,18 +49,16 @@ static void efx_mtd_remove_partition(struct efx_mtd_partition *part)
 		ssleep(1);
 	}
 	WARN_ON(rc);
-	list_del(&part->node);
 }
 
-int efx_mtd_add(struct efx_nic *efx, struct efx_mtd_partition *parts,
-		size_t n_parts, size_t sizeof_part)
+int efx_mtd_add(struct efx_nic *efx, struct efx_mcdi_mtd_partition *parts,
+		size_t n_parts)
 {
 	struct efx_mtd_partition *part;
 	size_t i;
 
 	for (i = 0; i < n_parts; i++) {
-		part = (struct efx_mtd_partition *)((char *)parts +
-						    i * sizeof_part);
+		part = &parts[i].common;
 
 		part->mtd.writesize = 1;
 
@@ -78,47 +77,38 @@ int efx_mtd_add(struct efx_nic *efx, struct efx_mtd_partition *parts,
 
 		if (mtd_device_register(&part->mtd, NULL, 0))
 			goto fail;
-
-		/* Add to list in order - efx_mtd_remove() depends on this */
-		list_add_tail(&part->node, &efx->mtd_list);
 	}
 
 	return 0;
 
 fail:
-	while (i--) {
-		part = (struct efx_mtd_partition *)((char *)parts +
-						    i * sizeof_part);
-		efx_mtd_remove_partition(part);
-	}
+	while (i--)
+		efx_mtd_remove_partition(&parts[i].common);
+
 	/* Failure is unlikely here, but probably means we're out of memory */
 	return -ENOMEM;
 }
 
 void efx_mtd_remove(struct efx_nic *efx)
 {
-	struct efx_mtd_partition *parts, *part, *next;
+	int i;
 
 	WARN_ON(efx_dev_registered(efx));
 
-	if (list_empty(&efx->mtd_list))
-		return;
-
-	parts = list_first_entry(&efx->mtd_list, struct efx_mtd_partition,
-				 node);
+	for (i = 0; i < efx->n_mcdi_mtd_parts; i++)
+		efx_mtd_remove_partition(&efx->mcdi_mtd_parts[i].common);
 
-	list_for_each_entry_safe(part, next, &efx->mtd_list, node)
-		efx_mtd_remove_partition(part);
-
-	kfree(parts);
+	kfree(efx->mcdi_mtd_parts);
+	efx->mcdi_mtd_parts = NULL;
+	efx->n_mcdi_mtd_parts = 0;
 }
 
 void efx_mtd_rename(struct efx_nic *efx)
 {
-	struct efx_mtd_partition *part;
+	int i;
 
 	ASSERT_RTNL();
 
-	list_for_each_entry(part, &efx->mtd_list, node)
-		efx->type->mtd_rename(part);
+	for (i = 0; i < efx->n_mcdi_mtd_parts; i++)
+		efx->type->mtd_rename(&efx->mcdi_mtd_parts[i].common);
 }
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index 318db906a154..5d20b25b0e82 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -107,6 +107,8 @@ struct hwtstamp_config;
 
 struct efx_self_tests;
 
+struct efx_mcdi_mtd_partition;
+
 /**
  * struct efx_buffer - A general-purpose DMA buffer
  * @addr: host base address of the buffer
@@ -865,7 +867,8 @@ enum efx_xdp_tx_queues_mode {
  * @irq_zero_count: Number of legacy IRQs seen with queue flags == 0
  * @irq_level: IRQ level/index for IRQs not triggered by an event queue
  * @selftest_work: Work item for asynchronous self-test
- * @mtd_list: List of MTDs attached to the NIC
+ * @mcdi_mtd_parts: Array of MTDs attached to the NIC
+ * @n_mcdi_mtd_parts: Number of MTDs attached to the NIC
  * @nic_data: Hardware dependent state
  * @mcdi: Management-Controller-to-Driver Interface state
  * @mac_lock: MAC access lock. Protects @port_enabled, @phy_mode,
@@ -1033,7 +1036,8 @@ struct efx_nic {
 	struct delayed_work selftest_work;
 
 #ifdef CONFIG_SFC_MTD
-	struct list_head mtd_list;
+	struct efx_mcdi_mtd_partition *mcdi_mtd_parts;
+	unsigned int n_mcdi_mtd_parts;
 #endif
 
 	void *nic_data;
@@ -1134,7 +1138,6 @@ static inline unsigned int efx_port_num(struct efx_nic *efx)
 }
 
 struct efx_mtd_partition {
-	struct list_head node;
 	struct mtd_info mtd;
 	const char *dev_type_name;
 	const char *type_name;
-- 
2.34.1


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

* [PATCH net-next v2 2/2] sfc/siena: simplify mtd partitions list handling
  2022-05-24  6:22 [PATCH net-next v2 0/2] sfc: simplify mtd partitions list handling Íñigo Huguet
  2022-05-24  6:22 ` [PATCH net-next v2 1/2] " Íñigo Huguet
@ 2022-05-24  6:22 ` Íñigo Huguet
  2022-05-27  6:18   ` Martin Habets
  2022-05-24 10:17 ` [PATCH net-next v2 0/2] sfc: " Paolo Abeni
  2 siblings, 1 reply; 6+ messages in thread
From: Íñigo Huguet @ 2022-05-24  6:22 UTC (permalink / raw)
  To: ecree.xilinx, habetsm.xilinx
  Cc: davem, edumazet, kuba, pabeni, netdev, Íñigo Huguet

Like in previous patch, get rid of efx->mtd_list and use the allocated
array of efx_mcdi_mtd_partition instead, also in siena.

Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
---
 drivers/net/ethernet/sfc/siena/efx.h        |  4 +-
 drivers/net/ethernet/sfc/siena/efx_common.c |  3 --
 drivers/net/ethernet/sfc/siena/mtd.c        | 42 ++++++++-------------
 drivers/net/ethernet/sfc/siena/net_driver.h |  9 +++--
 drivers/net/ethernet/sfc/siena/siena.c      | 12 ++++--
 5 files changed, 33 insertions(+), 37 deletions(-)

diff --git a/drivers/net/ethernet/sfc/siena/efx.h b/drivers/net/ethernet/sfc/siena/efx.h
index 27d1d3f19cae..30ef11dfa7a1 100644
--- a/drivers/net/ethernet/sfc/siena/efx.h
+++ b/drivers/net/ethernet/sfc/siena/efx.h
@@ -163,8 +163,8 @@ void efx_siena_update_sw_stats(struct efx_nic *efx, u64 *stats);
 
 /* MTD */
 #ifdef CONFIG_SFC_SIENA_MTD
-int efx_siena_mtd_add(struct efx_nic *efx, struct efx_mtd_partition *parts,
-		      size_t n_parts, size_t sizeof_part);
+int efx_siena_mtd_add(struct efx_nic *efx, struct efx_mcdi_mtd_partition *parts,
+		      size_t n_parts);
 static inline int efx_mtd_probe(struct efx_nic *efx)
 {
 	return efx->type->mtd_probe(efx);
diff --git a/drivers/net/ethernet/sfc/siena/efx_common.c b/drivers/net/ethernet/sfc/siena/efx_common.c
index 954daf464abb..dbf48d682684 100644
--- a/drivers/net/ethernet/sfc/siena/efx_common.c
+++ b/drivers/net/ethernet/sfc/siena/efx_common.c
@@ -997,9 +997,6 @@ int efx_siena_init_struct(struct efx_nic *efx,
 	INIT_LIST_HEAD(&efx->node);
 	INIT_LIST_HEAD(&efx->secondary_list);
 	spin_lock_init(&efx->biu_lock);
-#ifdef CONFIG_SFC_SIENA_MTD
-	INIT_LIST_HEAD(&efx->mtd_list);
-#endif
 	INIT_WORK(&efx->reset_work, efx_reset_work);
 	INIT_DELAYED_WORK(&efx->monitor_work, efx_monitor);
 	efx_siena_selftest_async_init(efx);
diff --git a/drivers/net/ethernet/sfc/siena/mtd.c b/drivers/net/ethernet/sfc/siena/mtd.c
index 12a624247f44..d6700822c6fa 100644
--- a/drivers/net/ethernet/sfc/siena/mtd.c
+++ b/drivers/net/ethernet/sfc/siena/mtd.c
@@ -12,6 +12,7 @@
 
 #include "net_driver.h"
 #include "efx.h"
+#include "mcdi.h"
 
 #define to_efx_mtd_partition(mtd)				\
 	container_of(mtd, struct efx_mtd_partition, mtd)
@@ -48,18 +49,16 @@ static void efx_siena_mtd_remove_partition(struct efx_mtd_partition *part)
 		ssleep(1);
 	}
 	WARN_ON(rc);
-	list_del(&part->node);
 }
 
-int efx_siena_mtd_add(struct efx_nic *efx, struct efx_mtd_partition *parts,
-		      size_t n_parts, size_t sizeof_part)
+int efx_siena_mtd_add(struct efx_nic *efx, struct efx_mcdi_mtd_partition *parts,
+		      size_t n_parts)
 {
 	struct efx_mtd_partition *part;
 	size_t i;
 
 	for (i = 0; i < n_parts; i++) {
-		part = (struct efx_mtd_partition *)((char *)parts +
-						    i * sizeof_part);
+		part = &parts[i].common;
 
 		part->mtd.writesize = 1;
 
@@ -78,47 +77,38 @@ int efx_siena_mtd_add(struct efx_nic *efx, struct efx_mtd_partition *parts,
 
 		if (mtd_device_register(&part->mtd, NULL, 0))
 			goto fail;
-
-		/* Add to list in order - efx_siena_mtd_remove() depends on this */
-		list_add_tail(&part->node, &efx->mtd_list);
 	}
 
 	return 0;
 
 fail:
-	while (i--) {
-		part = (struct efx_mtd_partition *)((char *)parts +
-						    i * sizeof_part);
-		efx_siena_mtd_remove_partition(part);
-	}
+	while (i--)
+		efx_siena_mtd_remove_partition(&parts[i].common);
+
 	/* Failure is unlikely here, but probably means we're out of memory */
 	return -ENOMEM;
 }
 
 void efx_siena_mtd_remove(struct efx_nic *efx)
 {
-	struct efx_mtd_partition *parts, *part, *next;
+	int i;
 
 	WARN_ON(efx_dev_registered(efx));
 
-	if (list_empty(&efx->mtd_list))
-		return;
-
-	parts = list_first_entry(&efx->mtd_list, struct efx_mtd_partition,
-				 node);
+	for (i = 0; i < efx->n_mcdi_mtd_parts; i++)
+		efx_siena_mtd_remove_partition(&efx->mcdi_mtd_parts[i].common);
 
-	list_for_each_entry_safe(part, next, &efx->mtd_list, node)
-		efx_siena_mtd_remove_partition(part);
-
-	kfree(parts);
+	kfree(efx->mcdi_mtd_parts);
+	efx->mcdi_mtd_parts = NULL;
+	efx->n_mcdi_mtd_parts = 0;
 }
 
 void efx_siena_mtd_rename(struct efx_nic *efx)
 {
-	struct efx_mtd_partition *part;
+	int i;
 
 	ASSERT_RTNL();
 
-	list_for_each_entry(part, &efx->mtd_list, node)
-		efx->type->mtd_rename(part);
+	for (i = 0; i < efx->n_mcdi_mtd_parts; i++)
+		efx->type->mtd_rename(&efx->mcdi_mtd_parts[i].common);
 }
diff --git a/drivers/net/ethernet/sfc/siena/net_driver.h b/drivers/net/ethernet/sfc/siena/net_driver.h
index a8f6c3699c8b..4c614f079359 100644
--- a/drivers/net/ethernet/sfc/siena/net_driver.h
+++ b/drivers/net/ethernet/sfc/siena/net_driver.h
@@ -107,6 +107,8 @@ struct hwtstamp_config;
 
 struct efx_self_tests;
 
+struct efx_mcdi_mtd_partition;
+
 /**
  * struct efx_buffer - A general-purpose DMA buffer
  * @addr: host base address of the buffer
@@ -864,7 +866,8 @@ enum efx_xdp_tx_queues_mode {
  * @irq_zero_count: Number of legacy IRQs seen with queue flags == 0
  * @irq_level: IRQ level/index for IRQs not triggered by an event queue
  * @selftest_work: Work item for asynchronous self-test
- * @mtd_list: List of MTDs attached to the NIC
+ * @mcdi_mtd_parts: Array of MTDs attached to the NIC
+ * @n_mcdi_mtd_parts: Number of MTDs attached to the NIC
  * @nic_data: Hardware dependent state
  * @mcdi: Management-Controller-to-Driver Interface state
  * @mac_lock: MAC access lock. Protects @port_enabled, @phy_mode,
@@ -1032,7 +1035,8 @@ struct efx_nic {
 	struct delayed_work selftest_work;
 
 #ifdef CONFIG_SFC_SIENA_MTD
-	struct list_head mtd_list;
+	struct efx_mcdi_mtd_partition *mcdi_mtd_parts;
+	unsigned int n_mcdi_mtd_parts;
 #endif
 
 	void *nic_data;
@@ -1133,7 +1137,6 @@ static inline unsigned int efx_port_num(struct efx_nic *efx)
 }
 
 struct efx_mtd_partition {
-	struct list_head node;
 	struct mtd_info mtd;
 	const char *dev_type_name;
 	const char *type_name;
diff --git a/drivers/net/ethernet/sfc/siena/siena.c b/drivers/net/ethernet/sfc/siena/siena.c
index a44c8fa25748..c9431955792e 100644
--- a/drivers/net/ethernet/sfc/siena/siena.c
+++ b/drivers/net/ethernet/sfc/siena/siena.c
@@ -947,10 +947,16 @@ static int siena_mtd_probe(struct efx_nic *efx)
 	if (rc)
 		goto fail;
 
-	rc = efx_siena_mtd_add(efx, &parts[0].common, n_parts, sizeof(*parts));
-fail:
+	rc = efx_siena_mtd_add(efx, parts, n_parts);
 	if (rc)
-		kfree(parts);
+		goto fail;
+	efx->mcdi_mtd_parts = parts;
+	efx->n_mcdi_mtd_parts = n_parts;
+
+	return 0;
+
+fail:
+	kfree(parts);
 	return rc;
 }
 
-- 
2.34.1


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

* Re: [PATCH net-next v2 0/2] sfc: simplify mtd partitions list handling
  2022-05-24  6:22 [PATCH net-next v2 0/2] sfc: simplify mtd partitions list handling Íñigo Huguet
  2022-05-24  6:22 ` [PATCH net-next v2 1/2] " Íñigo Huguet
  2022-05-24  6:22 ` [PATCH net-next v2 2/2] sfc/siena: " Íñigo Huguet
@ 2022-05-24 10:17 ` Paolo Abeni
  2 siblings, 0 replies; 6+ messages in thread
From: Paolo Abeni @ 2022-05-24 10:17 UTC (permalink / raw)
  To: Íñigo Huguet, ecree.xilinx, habetsm.xilinx
  Cc: davem, edumazet, kuba, netdev

Hello,

On Tue, 2022-05-24 at 08:22 +0200, Íñigo Huguet wrote:
> Simplify the mtd list handling to make more clear how it works and avoid
> potential future problems.
> 
> Tested: module load and unload in a system with Siena and EF10 cards
> installed, both modules sfc and sfc_siena.
> 
> v2:
> * Dropped patch fixing memory leak, already fixed in net
> * Apply changes also in new siena driver
> 
> Íñigo Huguet (2):
>   sfc: simplify mtd partitions list handling
>   sfc/siena: simplify mtd partitions list handling
> 
>  drivers/net/ethernet/sfc/ef10.c             | 12 ++++--
>  drivers/net/ethernet/sfc/efx.h              |  4 +-
>  drivers/net/ethernet/sfc/efx_common.c       |  3 --
>  drivers/net/ethernet/sfc/mtd.c              | 42 ++++++++-------------
>  drivers/net/ethernet/sfc/net_driver.h       |  9 +++--
>  drivers/net/ethernet/sfc/siena/efx.h        |  4 +-
>  drivers/net/ethernet/sfc/siena/efx_common.c |  3 --
>  drivers/net/ethernet/sfc/siena/mtd.c        | 42 ++++++++-------------
>  drivers/net/ethernet/sfc/siena/net_driver.h |  9 +++--
>  drivers/net/ethernet/sfc/siena/siena.c      | 12 ++++--
>  10 files changed, 66 insertions(+), 74 deletions(-)
> 
net-next is currently closed. Please wait untill it reopens and re-
post.

thanks!

Paolo


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

* Re: [PATCH net-next v2 1/2] sfc: simplify mtd partitions list handling
  2022-05-24  6:22 ` [PATCH net-next v2 1/2] " Íñigo Huguet
@ 2022-05-27  6:17   ` Martin Habets
  0 siblings, 0 replies; 6+ messages in thread
From: Martin Habets @ 2022-05-27  6:17 UTC (permalink / raw)
  To: Íñigo Huguet
  Cc: ecree.xilinx, davem, edumazet, kuba, pabeni, netdev

On Tue, May 24, 2022 at 08:22:42AM +0200, Íñigo Huguet wrote:
> efx_mtd_partitions are embedded inside efx_mcdi_mtd_partition structs.
> They contain a list entry that is appended to efx->mtd_list, which is
> traversed to perform add/remove/rename/etc operations over all
> efx_mtd_partitions.
> 
> However, almost all operations done on a efx_mtd_partition asume that it
> is actually embedded inside an efx_mcdi_mtd_partition, and the
> deallocation asume that the first member of the list is located at the
> beginning of the allocated memory.
> 
> Given all that asumptions, the possibility of having an
> efx_mtd_partition not embedded in an efx_mcdi_efx_partition doesn't
> exist. Neither it does the possibility of being in a memory position
> other the one allocated for the efx_mcdi_mtd_partition array. Also, they
> never need to be reordered.
> 
> Given all that, it is better to get rid of the list and use directly the
> efx_mcdi_mtd_partition array. This shows more clearly how they lay
> in memory, list traversal is more obvious and it save a small amount
> of memory on the list nodes.
> 
> Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>

Acked-by: Martin Habets <habetsm.xilinx@gmail.com>

> ---
>  drivers/net/ethernet/sfc/ef10.c       | 12 ++++++--
>  drivers/net/ethernet/sfc/efx.h        |  4 +--
>  drivers/net/ethernet/sfc/efx_common.c |  3 --
>  drivers/net/ethernet/sfc/mtd.c        | 42 ++++++++++-----------------
>  drivers/net/ethernet/sfc/net_driver.h |  9 ++++--
>  5 files changed, 33 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c
> index 186cb28c03bd..89f8c1e63ec0 100644
> --- a/drivers/net/ethernet/sfc/ef10.c
> +++ b/drivers/net/ethernet/sfc/ef10.c
> @@ -3584,10 +3584,16 @@ static int efx_ef10_mtd_probe(struct efx_nic *efx)
>  		return 0;
>  	}
>  
> -	rc = efx_mtd_add(efx, &parts[0].common, n_parts, sizeof(*parts));
> -fail:
> +	rc = efx_mtd_add(efx, parts, n_parts);
>  	if (rc)
> -		kfree(parts);
> +		goto fail;
> +	efx->mcdi_mtd_parts = parts;
> +	efx->n_mcdi_mtd_parts = n_parts;
> +
> +	return 0;
> +
> +fail:
> +	kfree(parts);
>  	return rc;
>  }
>  
> diff --git a/drivers/net/ethernet/sfc/efx.h b/drivers/net/ethernet/sfc/efx.h
> index c05a83da9e44..2ab9ba691b0d 100644
> --- a/drivers/net/ethernet/sfc/efx.h
> +++ b/drivers/net/ethernet/sfc/efx.h
> @@ -181,8 +181,8 @@ void efx_update_sw_stats(struct efx_nic *efx, u64 *stats);
>  
>  /* MTD */
>  #ifdef CONFIG_SFC_MTD
> -int efx_mtd_add(struct efx_nic *efx, struct efx_mtd_partition *parts,
> -		size_t n_parts, size_t sizeof_part);
> +int efx_mtd_add(struct efx_nic *efx, struct efx_mcdi_mtd_partition *parts,
> +		size_t n_parts);
>  static inline int efx_mtd_probe(struct efx_nic *efx)
>  {
>  	return efx->type->mtd_probe(efx);
> diff --git a/drivers/net/ethernet/sfc/efx_common.c b/drivers/net/ethernet/sfc/efx_common.c
> index f6577e74d6e6..8802790403e9 100644
> --- a/drivers/net/ethernet/sfc/efx_common.c
> +++ b/drivers/net/ethernet/sfc/efx_common.c
> @@ -987,9 +987,6 @@ int efx_init_struct(struct efx_nic *efx,
>  	INIT_LIST_HEAD(&efx->node);
>  	INIT_LIST_HEAD(&efx->secondary_list);
>  	spin_lock_init(&efx->biu_lock);
> -#ifdef CONFIG_SFC_MTD
> -	INIT_LIST_HEAD(&efx->mtd_list);
> -#endif
>  	INIT_WORK(&efx->reset_work, efx_reset_work);
>  	INIT_DELAYED_WORK(&efx->monitor_work, efx_monitor);
>  	efx_selftest_async_init(efx);
> diff --git a/drivers/net/ethernet/sfc/mtd.c b/drivers/net/ethernet/sfc/mtd.c
> index 273c08e5455f..4d06e8a9a729 100644
> --- a/drivers/net/ethernet/sfc/mtd.c
> +++ b/drivers/net/ethernet/sfc/mtd.c
> @@ -12,6 +12,7 @@
>  
>  #include "net_driver.h"
>  #include "efx.h"
> +#include "mcdi.h"
>  
>  #define to_efx_mtd_partition(mtd)				\
>  	container_of(mtd, struct efx_mtd_partition, mtd)
> @@ -48,18 +49,16 @@ static void efx_mtd_remove_partition(struct efx_mtd_partition *part)
>  		ssleep(1);
>  	}
>  	WARN_ON(rc);
> -	list_del(&part->node);
>  }
>  
> -int efx_mtd_add(struct efx_nic *efx, struct efx_mtd_partition *parts,
> -		size_t n_parts, size_t sizeof_part)
> +int efx_mtd_add(struct efx_nic *efx, struct efx_mcdi_mtd_partition *parts,
> +		size_t n_parts)
>  {
>  	struct efx_mtd_partition *part;
>  	size_t i;
>  
>  	for (i = 0; i < n_parts; i++) {
> -		part = (struct efx_mtd_partition *)((char *)parts +
> -						    i * sizeof_part);
> +		part = &parts[i].common;
>  
>  		part->mtd.writesize = 1;
>  
> @@ -78,47 +77,38 @@ int efx_mtd_add(struct efx_nic *efx, struct efx_mtd_partition *parts,
>  
>  		if (mtd_device_register(&part->mtd, NULL, 0))
>  			goto fail;
> -
> -		/* Add to list in order - efx_mtd_remove() depends on this */
> -		list_add_tail(&part->node, &efx->mtd_list);
>  	}
>  
>  	return 0;
>  
>  fail:
> -	while (i--) {
> -		part = (struct efx_mtd_partition *)((char *)parts +
> -						    i * sizeof_part);
> -		efx_mtd_remove_partition(part);
> -	}
> +	while (i--)
> +		efx_mtd_remove_partition(&parts[i].common);
> +
>  	/* Failure is unlikely here, but probably means we're out of memory */
>  	return -ENOMEM;
>  }
>  
>  void efx_mtd_remove(struct efx_nic *efx)
>  {
> -	struct efx_mtd_partition *parts, *part, *next;
> +	int i;
>  
>  	WARN_ON(efx_dev_registered(efx));
>  
> -	if (list_empty(&efx->mtd_list))
> -		return;
> -
> -	parts = list_first_entry(&efx->mtd_list, struct efx_mtd_partition,
> -				 node);
> +	for (i = 0; i < efx->n_mcdi_mtd_parts; i++)
> +		efx_mtd_remove_partition(&efx->mcdi_mtd_parts[i].common);
>  
> -	list_for_each_entry_safe(part, next, &efx->mtd_list, node)
> -		efx_mtd_remove_partition(part);
> -
> -	kfree(parts);
> +	kfree(efx->mcdi_mtd_parts);
> +	efx->mcdi_mtd_parts = NULL;
> +	efx->n_mcdi_mtd_parts = 0;
>  }
>  
>  void efx_mtd_rename(struct efx_nic *efx)
>  {
> -	struct efx_mtd_partition *part;
> +	int i;
>  
>  	ASSERT_RTNL();
>  
> -	list_for_each_entry(part, &efx->mtd_list, node)
> -		efx->type->mtd_rename(part);
> +	for (i = 0; i < efx->n_mcdi_mtd_parts; i++)
> +		efx->type->mtd_rename(&efx->mcdi_mtd_parts[i].common);
>  }
> diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
> index 318db906a154..5d20b25b0e82 100644
> --- a/drivers/net/ethernet/sfc/net_driver.h
> +++ b/drivers/net/ethernet/sfc/net_driver.h
> @@ -107,6 +107,8 @@ struct hwtstamp_config;
>  
>  struct efx_self_tests;
>  
> +struct efx_mcdi_mtd_partition;
> +
>  /**
>   * struct efx_buffer - A general-purpose DMA buffer
>   * @addr: host base address of the buffer
> @@ -865,7 +867,8 @@ enum efx_xdp_tx_queues_mode {
>   * @irq_zero_count: Number of legacy IRQs seen with queue flags == 0
>   * @irq_level: IRQ level/index for IRQs not triggered by an event queue
>   * @selftest_work: Work item for asynchronous self-test
> - * @mtd_list: List of MTDs attached to the NIC
> + * @mcdi_mtd_parts: Array of MTDs attached to the NIC
> + * @n_mcdi_mtd_parts: Number of MTDs attached to the NIC
>   * @nic_data: Hardware dependent state
>   * @mcdi: Management-Controller-to-Driver Interface state
>   * @mac_lock: MAC access lock. Protects @port_enabled, @phy_mode,
> @@ -1033,7 +1036,8 @@ struct efx_nic {
>  	struct delayed_work selftest_work;
>  
>  #ifdef CONFIG_SFC_MTD
> -	struct list_head mtd_list;
> +	struct efx_mcdi_mtd_partition *mcdi_mtd_parts;
> +	unsigned int n_mcdi_mtd_parts;
>  #endif
>  
>  	void *nic_data;
> @@ -1134,7 +1138,6 @@ static inline unsigned int efx_port_num(struct efx_nic *efx)
>  }
>  
>  struct efx_mtd_partition {
> -	struct list_head node;
>  	struct mtd_info mtd;
>  	const char *dev_type_name;
>  	const char *type_name;
> -- 
> 2.34.1

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

* Re: [PATCH net-next v2 2/2] sfc/siena: simplify mtd partitions list handling
  2022-05-24  6:22 ` [PATCH net-next v2 2/2] sfc/siena: " Íñigo Huguet
@ 2022-05-27  6:18   ` Martin Habets
  0 siblings, 0 replies; 6+ messages in thread
From: Martin Habets @ 2022-05-27  6:18 UTC (permalink / raw)
  To: Íñigo Huguet
  Cc: ecree.xilinx, davem, edumazet, kuba, pabeni, netdev

On Tue, May 24, 2022 at 08:22:43AM +0200, Íñigo Huguet wrote:
> Like in previous patch, get rid of efx->mtd_list and use the allocated
> array of efx_mcdi_mtd_partition instead, also in siena.
> 
> Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>

Acked-by: Martin Habets <habetsm.xilinx@gmail.com>

> ---
>  drivers/net/ethernet/sfc/siena/efx.h        |  4 +-
>  drivers/net/ethernet/sfc/siena/efx_common.c |  3 --
>  drivers/net/ethernet/sfc/siena/mtd.c        | 42 ++++++++-------------
>  drivers/net/ethernet/sfc/siena/net_driver.h |  9 +++--
>  drivers/net/ethernet/sfc/siena/siena.c      | 12 ++++--
>  5 files changed, 33 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/siena/efx.h b/drivers/net/ethernet/sfc/siena/efx.h
> index 27d1d3f19cae..30ef11dfa7a1 100644
> --- a/drivers/net/ethernet/sfc/siena/efx.h
> +++ b/drivers/net/ethernet/sfc/siena/efx.h
> @@ -163,8 +163,8 @@ void efx_siena_update_sw_stats(struct efx_nic *efx, u64 *stats);
>  
>  /* MTD */
>  #ifdef CONFIG_SFC_SIENA_MTD
> -int efx_siena_mtd_add(struct efx_nic *efx, struct efx_mtd_partition *parts,
> -		      size_t n_parts, size_t sizeof_part);
> +int efx_siena_mtd_add(struct efx_nic *efx, struct efx_mcdi_mtd_partition *parts,
> +		      size_t n_parts);
>  static inline int efx_mtd_probe(struct efx_nic *efx)
>  {
>  	return efx->type->mtd_probe(efx);
> diff --git a/drivers/net/ethernet/sfc/siena/efx_common.c b/drivers/net/ethernet/sfc/siena/efx_common.c
> index 954daf464abb..dbf48d682684 100644
> --- a/drivers/net/ethernet/sfc/siena/efx_common.c
> +++ b/drivers/net/ethernet/sfc/siena/efx_common.c
> @@ -997,9 +997,6 @@ int efx_siena_init_struct(struct efx_nic *efx,
>  	INIT_LIST_HEAD(&efx->node);
>  	INIT_LIST_HEAD(&efx->secondary_list);
>  	spin_lock_init(&efx->biu_lock);
> -#ifdef CONFIG_SFC_SIENA_MTD
> -	INIT_LIST_HEAD(&efx->mtd_list);
> -#endif
>  	INIT_WORK(&efx->reset_work, efx_reset_work);
>  	INIT_DELAYED_WORK(&efx->monitor_work, efx_monitor);
>  	efx_siena_selftest_async_init(efx);
> diff --git a/drivers/net/ethernet/sfc/siena/mtd.c b/drivers/net/ethernet/sfc/siena/mtd.c
> index 12a624247f44..d6700822c6fa 100644
> --- a/drivers/net/ethernet/sfc/siena/mtd.c
> +++ b/drivers/net/ethernet/sfc/siena/mtd.c
> @@ -12,6 +12,7 @@
>  
>  #include "net_driver.h"
>  #include "efx.h"
> +#include "mcdi.h"
>  
>  #define to_efx_mtd_partition(mtd)				\
>  	container_of(mtd, struct efx_mtd_partition, mtd)
> @@ -48,18 +49,16 @@ static void efx_siena_mtd_remove_partition(struct efx_mtd_partition *part)
>  		ssleep(1);
>  	}
>  	WARN_ON(rc);
> -	list_del(&part->node);
>  }
>  
> -int efx_siena_mtd_add(struct efx_nic *efx, struct efx_mtd_partition *parts,
> -		      size_t n_parts, size_t sizeof_part)
> +int efx_siena_mtd_add(struct efx_nic *efx, struct efx_mcdi_mtd_partition *parts,
> +		      size_t n_parts)
>  {
>  	struct efx_mtd_partition *part;
>  	size_t i;
>  
>  	for (i = 0; i < n_parts; i++) {
> -		part = (struct efx_mtd_partition *)((char *)parts +
> -						    i * sizeof_part);
> +		part = &parts[i].common;
>  
>  		part->mtd.writesize = 1;
>  
> @@ -78,47 +77,38 @@ int efx_siena_mtd_add(struct efx_nic *efx, struct efx_mtd_partition *parts,
>  
>  		if (mtd_device_register(&part->mtd, NULL, 0))
>  			goto fail;
> -
> -		/* Add to list in order - efx_siena_mtd_remove() depends on this */
> -		list_add_tail(&part->node, &efx->mtd_list);
>  	}
>  
>  	return 0;
>  
>  fail:
> -	while (i--) {
> -		part = (struct efx_mtd_partition *)((char *)parts +
> -						    i * sizeof_part);
> -		efx_siena_mtd_remove_partition(part);
> -	}
> +	while (i--)
> +		efx_siena_mtd_remove_partition(&parts[i].common);
> +
>  	/* Failure is unlikely here, but probably means we're out of memory */
>  	return -ENOMEM;
>  }
>  
>  void efx_siena_mtd_remove(struct efx_nic *efx)
>  {
> -	struct efx_mtd_partition *parts, *part, *next;
> +	int i;
>  
>  	WARN_ON(efx_dev_registered(efx));
>  
> -	if (list_empty(&efx->mtd_list))
> -		return;
> -
> -	parts = list_first_entry(&efx->mtd_list, struct efx_mtd_partition,
> -				 node);
> +	for (i = 0; i < efx->n_mcdi_mtd_parts; i++)
> +		efx_siena_mtd_remove_partition(&efx->mcdi_mtd_parts[i].common);
>  
> -	list_for_each_entry_safe(part, next, &efx->mtd_list, node)
> -		efx_siena_mtd_remove_partition(part);
> -
> -	kfree(parts);
> +	kfree(efx->mcdi_mtd_parts);
> +	efx->mcdi_mtd_parts = NULL;
> +	efx->n_mcdi_mtd_parts = 0;
>  }
>  
>  void efx_siena_mtd_rename(struct efx_nic *efx)
>  {
> -	struct efx_mtd_partition *part;
> +	int i;
>  
>  	ASSERT_RTNL();
>  
> -	list_for_each_entry(part, &efx->mtd_list, node)
> -		efx->type->mtd_rename(part);
> +	for (i = 0; i < efx->n_mcdi_mtd_parts; i++)
> +		efx->type->mtd_rename(&efx->mcdi_mtd_parts[i].common);
>  }
> diff --git a/drivers/net/ethernet/sfc/siena/net_driver.h b/drivers/net/ethernet/sfc/siena/net_driver.h
> index a8f6c3699c8b..4c614f079359 100644
> --- a/drivers/net/ethernet/sfc/siena/net_driver.h
> +++ b/drivers/net/ethernet/sfc/siena/net_driver.h
> @@ -107,6 +107,8 @@ struct hwtstamp_config;
>  
>  struct efx_self_tests;
>  
> +struct efx_mcdi_mtd_partition;
> +
>  /**
>   * struct efx_buffer - A general-purpose DMA buffer
>   * @addr: host base address of the buffer
> @@ -864,7 +866,8 @@ enum efx_xdp_tx_queues_mode {
>   * @irq_zero_count: Number of legacy IRQs seen with queue flags == 0
>   * @irq_level: IRQ level/index for IRQs not triggered by an event queue
>   * @selftest_work: Work item for asynchronous self-test
> - * @mtd_list: List of MTDs attached to the NIC
> + * @mcdi_mtd_parts: Array of MTDs attached to the NIC
> + * @n_mcdi_mtd_parts: Number of MTDs attached to the NIC
>   * @nic_data: Hardware dependent state
>   * @mcdi: Management-Controller-to-Driver Interface state
>   * @mac_lock: MAC access lock. Protects @port_enabled, @phy_mode,
> @@ -1032,7 +1035,8 @@ struct efx_nic {
>  	struct delayed_work selftest_work;
>  
>  #ifdef CONFIG_SFC_SIENA_MTD
> -	struct list_head mtd_list;
> +	struct efx_mcdi_mtd_partition *mcdi_mtd_parts;
> +	unsigned int n_mcdi_mtd_parts;
>  #endif
>  
>  	void *nic_data;
> @@ -1133,7 +1137,6 @@ static inline unsigned int efx_port_num(struct efx_nic *efx)
>  }
>  
>  struct efx_mtd_partition {
> -	struct list_head node;
>  	struct mtd_info mtd;
>  	const char *dev_type_name;
>  	const char *type_name;
> diff --git a/drivers/net/ethernet/sfc/siena/siena.c b/drivers/net/ethernet/sfc/siena/siena.c
> index a44c8fa25748..c9431955792e 100644
> --- a/drivers/net/ethernet/sfc/siena/siena.c
> +++ b/drivers/net/ethernet/sfc/siena/siena.c
> @@ -947,10 +947,16 @@ static int siena_mtd_probe(struct efx_nic *efx)
>  	if (rc)
>  		goto fail;
>  
> -	rc = efx_siena_mtd_add(efx, &parts[0].common, n_parts, sizeof(*parts));
> -fail:
> +	rc = efx_siena_mtd_add(efx, parts, n_parts);
>  	if (rc)
> -		kfree(parts);
> +		goto fail;
> +	efx->mcdi_mtd_parts = parts;
> +	efx->n_mcdi_mtd_parts = n_parts;
> +
> +	return 0;
> +
> +fail:
> +	kfree(parts);
>  	return rc;
>  }
>  
> -- 
> 2.34.1

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

end of thread, other threads:[~2022-05-27  6:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-24  6:22 [PATCH net-next v2 0/2] sfc: simplify mtd partitions list handling Íñigo Huguet
2022-05-24  6:22 ` [PATCH net-next v2 1/2] " Íñigo Huguet
2022-05-27  6:17   ` Martin Habets
2022-05-24  6:22 ` [PATCH net-next v2 2/2] sfc/siena: " Íñigo Huguet
2022-05-27  6:18   ` Martin Habets
2022-05-24 10:17 ` [PATCH net-next v2 0/2] sfc: " Paolo Abeni

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.