Currently if a software reset is attempted on an idxd device
the workqueues will not be re-enabled, because it will see
incorrectly see that wq->state is already set to IDXD_WQ_ENABLED.
So set the workqueue state to disabled in idxd_wq_disable_cleanup(),
and use a bitmap to track which workqueues have been enabled so they
can be re-enabled during device re-initialization.
Changes from v1 to v2:
- Clear bit in case where idxd_wq_enable() fails during re-init.
Cc: Fenghua Yu <[email protected]>
Cc: Dave Jiang <[email protected]>
Cc: Vinod Koul <[email protected]>
Jerry Snitselaar (2):
dmaengine: idxd: Set wq state to disabled in idxd_wq_disable_cleanup()
dmaengine: idxd: track enabled workqueues in bitmap
drivers/dma/idxd/device.c | 4 +++-
drivers/dma/idxd/idxd.h | 2 ++
drivers/dma/idxd/init.c | 6 ++++++
drivers/dma/idxd/irq.c | 5 +++--
drivers/dma/idxd/sysfs.c | 1 +
5 files changed, 15 insertions(+), 3 deletions(-)
--
2.37.2
If we are calling idxd_wq_disable_cleanup(), the workqueue should be
in a disabled state. So set the workqueue state to IDXD_WQ_DISABLED so
that the state reflects that. Currently if there is a device failure,
and a software reset is attempted the workqueues will not be
re-enabled due to idxd_wq_enable() seeing that state as already being
IDXD_WQ_ENABLED.
Cc: Fenghua Yu <[email protected]>
Cc: Dave Jiang <[email protected]>
Cc: Vinod Koul <[email protected]>
Signed-off-by: Jerry Snitselaar <[email protected]>
---
drivers/dma/idxd/device.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c
index 5a8cc52c1abf..31911e255ac1 100644
--- a/drivers/dma/idxd/device.c
+++ b/drivers/dma/idxd/device.c
@@ -258,7 +258,6 @@ void idxd_wq_reset(struct idxd_wq *wq)
operand = BIT(wq->id % 16) | ((wq->id / 16) << 16);
idxd_cmd_exec(idxd, IDXD_CMD_RESET_WQ, operand, NULL);
idxd_wq_disable_cleanup(wq);
- wq->state = IDXD_WQ_DISABLED;
}
int idxd_wq_map_portal(struct idxd_wq *wq)
@@ -378,6 +377,7 @@ static void idxd_wq_disable_cleanup(struct idxd_wq *wq)
struct idxd_device *idxd = wq->idxd;
lockdep_assert_held(&wq->wq_lock);
+ wq->state = IDXD_WQ_DISABLED;
memset(wq->wqcfg, 0, idxd->wqcfg_size);
wq->type = IDXD_WQT_NONE;
wq->threshold = 0;
--
2.37.2
Now that idxd_wq_disable_cleanup() sets the workqueue state to
IDXD_WQ_DISABLED, use a bitmap to track which workqueues have been
enabled. This will then be used to determine which workqueues
should be re-enabled when attempting a software reset to recover
from a device halt state.
Cc: Fenghua Yu <[email protected]>
Cc: Dave Jiang <[email protected]>
Cc: Vinod Koul <[email protected]>
Signed-off-by: Jerry Snitselaar <[email protected]>
---
v2: Clear bit in case where idxd_wq_enable fails during re-init
drivers/dma/idxd/device.c | 2 ++
drivers/dma/idxd/idxd.h | 2 ++
drivers/dma/idxd/init.c | 6 ++++++
drivers/dma/idxd/irq.c | 5 +++--
drivers/dma/idxd/sysfs.c | 1 +
5 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c
index 31911e255ac1..f0c7d6d348e3 100644
--- a/drivers/dma/idxd/device.c
+++ b/drivers/dma/idxd/device.c
@@ -196,6 +196,7 @@ int idxd_wq_enable(struct idxd_wq *wq)
}
wq->state = IDXD_WQ_ENABLED;
+ set_bit(wq->id, idxd->wq_enable_map);
dev_dbg(dev, "WQ %d enabled\n", wq->id);
return 0;
}
@@ -223,6 +224,7 @@ int idxd_wq_disable(struct idxd_wq *wq, bool reset_config)
if (reset_config)
idxd_wq_disable_cleanup(wq);
+ clear_bit(wq->id, idxd->wq_enable_map);
wq->state = IDXD_WQ_DISABLED;
dev_dbg(dev, "WQ %d disabled\n", wq->id);
return 0;
diff --git a/drivers/dma/idxd/idxd.h b/drivers/dma/idxd/idxd.h
index fed0dfc1eaa8..f527a7f88b92 100644
--- a/drivers/dma/idxd/idxd.h
+++ b/drivers/dma/idxd/idxd.h
@@ -11,6 +11,7 @@
#include <linux/idr.h>
#include <linux/pci.h>
#include <linux/ioasid.h>
+#include <linux/bitmap.h>
#include <linux/perf_event.h>
#include <uapi/linux/idxd.h>
#include "registers.h"
@@ -299,6 +300,7 @@ struct idxd_device {
int rdbuf_limit;
int nr_rdbufs; /* non-reserved read buffers */
unsigned int wqcfg_size;
+ unsigned long *wq_enable_map;
union sw_err_reg sw_err;
wait_queue_head_t cmd_waitq;
diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index aa3478257ddb..7e27e69ff741 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -151,6 +151,12 @@ static int idxd_setup_wqs(struct idxd_device *idxd)
if (!idxd->wqs)
return -ENOMEM;
+ idxd->wq_enable_map = bitmap_zalloc_node(idxd->max_wqs, GFP_KERNEL, dev_to_node(dev));
+ if (!idxd->wq_enable_map) {
+ kfree(idxd->wqs);
+ return -ENOMEM;
+ }
+
for (i = 0; i < idxd->max_wqs; i++) {
wq = kzalloc_node(sizeof(*wq), GFP_KERNEL, dev_to_node(dev));
if (!wq) {
diff --git a/drivers/dma/idxd/irq.c b/drivers/dma/idxd/irq.c
index 743ead5ebc57..3fcfbb7bf6e3 100644
--- a/drivers/dma/idxd/irq.c
+++ b/drivers/dma/idxd/irq.c
@@ -49,11 +49,12 @@ static void idxd_device_reinit(struct work_struct *work)
goto out;
for (i = 0; i < idxd->max_wqs; i++) {
- struct idxd_wq *wq = idxd->wqs[i];
+ if (test_bit(i, idxd->wq_enable_map)) {
+ struct idxd_wq *wq = idxd->wqs[i];
- if (wq->state == IDXD_WQ_ENABLED) {
rc = idxd_wq_enable(wq);
if (rc < 0) {
+ clear_bit(i, idxd->wq_enable_map);
dev_warn(dev, "Unable to re-enable wq %s\n",
dev_name(wq_confdev(wq)));
}
diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
index 3f262a57441b..3325b16ed959 100644
--- a/drivers/dma/idxd/sysfs.c
+++ b/drivers/dma/idxd/sysfs.c
@@ -1405,6 +1405,7 @@ static void idxd_conf_device_release(struct device *dev)
struct idxd_device *idxd = confdev_to_idxd(dev);
kfree(idxd->groups);
+ bitmap_free(idxd->wq_enable_map);
kfree(idxd->wqs);
kfree(idxd->engines);
ida_free(&idxd_ida, idxd->id);
--
2.37.2
On 28-09-22, 08:48, Jerry Snitselaar wrote:
> Currently if a software reset is attempted on an idxd device
> the workqueues will not be re-enabled, because it will see
> incorrectly see that wq->state is already set to IDXD_WQ_ENABLED.
> So set the workqueue state to disabled in idxd_wq_disable_cleanup(),
> and use a bitmap to track which workqueues have been enabled so they
> can be re-enabled during device re-initialization.
Applied, thanks
--
~Vinod