diff mbox series

raw/ioat: enable xstats reset for ioat device

Message ID 20191016131626.63781-1-ciara.power@intel.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series raw/ioat: enable xstats reset for ioat device |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-compilation success Compile Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Power, Ciara Oct. 16, 2019, 1:16 p.m. UTC 
  The rawdev xstats_reset function is now enabled.  It is called when the
ioat autotest completes, to reset all xstat values after they have been
modified during testing.

Signed-off-by: Ciara Power <ciara.power@intel.com>
---
 drivers/raw/ioat/ioat_rawdev.c      | 38 +++++++++++++++++++++++++++++
 drivers/raw/ioat/ioat_rawdev_test.c |  6 +++++
 2 files changed, 44 insertions(+)

Comments

Bruce Richardson Oct. 16, 2019, 1:36 p.m. UTC | #1 
On Wed, Oct 16, 2019 at 02:16:26PM +0100, Ciara Power wrote:
> The rawdev xstats_reset function is now enabled.  It is called when the
> ioat autotest completes, to reset all xstat values after they have been
> modified during testing.
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>

Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
David Marchand Oct. 27, 2019, 1:26 p.m. UTC | #2 
On Wed, Oct 16, 2019 at 3:36 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Wed, Oct 16, 2019 at 02:16:26PM +0100, Ciara Power wrote:
> > The rawdev xstats_reset function is now enabled.  It is called when the
> > ioat autotest completes, to reset all xstat values after they have been
> > modified during testing.
> >
> > Signed-off-by: Ciara Power <ciara.power@intel.com>
>
> Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>

This patch conflicts with the other series on this driver.
Is it really necessary to reset those statistics once the device has
been stopped?
Power, Ciara Oct. 29, 2019, 11:52 a.m. UTC | #3 
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Sunday 27 October 2019 13:27
> To: Power, Ciara <ciara.power@intel.com>
> Cc: dev <dev@dpdk.org>; Richardson, Bruce <bruce.richardson@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] raw/ioat: enable xstats reset for ioat device
> 
> On Wed, Oct 16, 2019 at 3:36 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > On Wed, Oct 16, 2019 at 02:16:26PM +0100, Ciara Power wrote:
> > > The rawdev xstats_reset function is now enabled.  It is called when
> > > the ioat autotest completes, to reset all xstat values after they
> > > have been modified during testing.
> > >
> > > Signed-off-by: Ciara Power <ciara.power@intel.com>
> >
> > Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> This patch conflicts with the other series on this driver.
> Is it really necessary to reset those statistics once the device has been
> stopped?
> 
> 
> --
> David Marchand

Hi David,

It is a necessary change because stopping the device does not reset the stats, which will result in them accumulating if the test is run multiple times.
Resetting the stats after stopping the device solves this problem.

I can rebase onto master to resolve the conflict, and send a new version for this patch if it helps.

Thanks,
- Ciara
David Marchand Oct. 29, 2019, 12:03 p.m. UTC | #4 
On Tue, Oct 29, 2019 at 12:52 PM Power, Ciara <ciara.power@intel.com> wrote:
> > This patch conflicts with the other series on this driver.
> > Is it really necessary to reset those statistics once the device has been
> > stopped?
> >> It is a necessary change because stopping the device does not reset the stats, which will result in them accumulating if the test is run multiple times.
> Resetting the stats after stopping the device solves this problem.

Ok, if this is intended.

> I can rebase onto master to resolve the conflict, and send a new version for this patch if it helps.

Yes please.
diff mbox series

Patch

diff --git a/drivers/raw/ioat/ioat_rawdev.c b/drivers/raw/ioat/ioat_rawdev.c
index 7270ad7aa..af8414b34 100644
--- a/drivers/raw/ioat/ioat_rawdev.c
+++ b/drivers/raw/ioat/ioat_rawdev.c
@@ -161,6 +161,43 @@  ioat_xstats_get_names(const struct rte_rawdev *dev,
 	return RTE_DIM(xstat_names);
 }
 
+static int
+ioat_xstats_reset(struct rte_rawdev *dev, const uint32_t *ids, uint32_t nb_ids)
+{
+	struct rte_ioat_rawdev *ioat = dev->dev_private;
+	unsigned int i;
+
+	if (!ids) {
+		ioat->enqueue_failed = 0;
+		ioat->enqueued = 0;
+		ioat->started = 0;
+		ioat->completed = 0;
+		return 0;
+	}
+
+	for (i = 0; i < nb_ids; i++) {
+		switch (ids[i]) {
+		case 0:
+			ioat->enqueue_failed = 0;
+			break;
+		case 1:
+			ioat->enqueued = 0;
+			break;
+		case 2:
+			ioat->started = 0;
+			break;
+		case 3:
+			ioat->completed = 0;
+			break;
+		default:
+			IOAT_PMD_WARN("Invalid xstat id - cannot reset value");
+			break;
+		}
+	}
+
+	return 0;
+}
+
 extern int ioat_rawdev_test(uint16_t dev_id);
 
 static int
@@ -173,6 +210,7 @@  ioat_rawdev_create(const char *name, struct rte_pci_device *dev)
 			.dev_info_get = ioat_dev_info_get,
 			.xstats_get = ioat_xstats_get,
 			.xstats_get_names = ioat_xstats_get_names,
+			.xstats_reset = ioat_xstats_reset,
 			.dev_selftest = ioat_rawdev_test,
 	};
 
diff --git a/drivers/raw/ioat/ioat_rawdev_test.c b/drivers/raw/ioat/ioat_rawdev_test.c
index f6c7dbb80..61d4bd066 100644
--- a/drivers/raw/ioat/ioat_rawdev_test.c
+++ b/drivers/raw/ioat/ioat_rawdev_test.c
@@ -220,6 +220,11 @@  ioat_rawdev_test(uint16_t dev_id)
 	}
 	printf("\n");
 
+	if (rte_rawdev_xstats_reset(dev_id, NULL, 0) != 0) {
+		printf("Error resetting xstat values\n");
+		goto err;
+	}
+
 	rte_mempool_free(pool);
 	free(snames);
 	free(stats);
@@ -227,6 +232,7 @@  ioat_rawdev_test(uint16_t dev_id)
 	return 0;
 
 err:
+	rte_rawdev_xstats_reset(dev_id, NULL, 0);
 	rte_mempool_free(pool);
 	free(snames);
 	free(stats);