* [PATCH v2 0/2] dmaengine: idxd: Fix up re-enabling device workqueues
@ 2022-09-28 15:48 Jerry Snitselaar
2022-09-28 15:48 ` [PATCH v2 1/2] dmaengine: idxd: Set wq state to disabled in idxd_wq_disable_cleanup() Jerry Snitselaar
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jerry Snitselaar @ 2022-09-28 15:48 UTC (permalink / raw)
To: linux-kernel, dmaengine; +Cc: Fenghua Yu, Dave Jiang, Vinod Koul
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 <fenghua.yu@intel.com>
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Vinod Koul <vkoul@kernel.org>
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] dmaengine: idxd: Set wq state to disabled in idxd_wq_disable_cleanup()
2022-09-28 15:48 [PATCH v2 0/2] dmaengine: idxd: Fix up re-enabling device workqueues Jerry Snitselaar
@ 2022-09-28 15:48 ` Jerry Snitselaar
2022-09-28 15:54 ` Dave Jiang
2022-09-28 15:48 ` [PATCH v2 2/2] dmaengine: idxd: track enabled workqueues in bitmap Jerry Snitselaar
2022-09-29 7:32 ` [PATCH v2 0/2] dmaengine: idxd: Fix up re-enabling device workqueues Vinod Koul
2 siblings, 1 reply; 6+ messages in thread
From: Jerry Snitselaar @ 2022-09-28 15:48 UTC (permalink / raw)
To: linux-kernel, dmaengine; +Cc: Fenghua Yu, Dave Jiang, Vinod Koul
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 <fenghua.yu@intel.com>
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] dmaengine: idxd: track enabled workqueues in bitmap
2022-09-28 15:48 [PATCH v2 0/2] dmaengine: idxd: Fix up re-enabling device workqueues Jerry Snitselaar
2022-09-28 15:48 ` [PATCH v2 1/2] dmaengine: idxd: Set wq state to disabled in idxd_wq_disable_cleanup() Jerry Snitselaar
@ 2022-09-28 15:48 ` Jerry Snitselaar
2022-09-28 15:51 ` Dave Jiang
2022-09-29 7:32 ` [PATCH v2 0/2] dmaengine: idxd: Fix up re-enabling device workqueues Vinod Koul
2 siblings, 1 reply; 6+ messages in thread
From: Jerry Snitselaar @ 2022-09-28 15:48 UTC (permalink / raw)
To: linux-kernel, dmaengine; +Cc: Fenghua Yu, Dave Jiang, Vinod Koul
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 <fenghua.yu@intel.com>
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] dmaengine: idxd: track enabled workqueues in bitmap
2022-09-28 15:48 ` [PATCH v2 2/2] dmaengine: idxd: track enabled workqueues in bitmap Jerry Snitselaar
@ 2022-09-28 15:51 ` Dave Jiang
0 siblings, 0 replies; 6+ messages in thread
From: Dave Jiang @ 2022-09-28 15:51 UTC (permalink / raw)
To: Jerry Snitselaar, linux-kernel, dmaengine; +Cc: Fenghua Yu, Vinod Koul
On 9/28/2022 8:48 AM, Jerry Snitselaar wrote:
> 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 <fenghua.yu@intel.com>
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Vinod Koul <vkoul@kernel.org>
> Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> 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);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] dmaengine: idxd: Set wq state to disabled in idxd_wq_disable_cleanup()
2022-09-28 15:48 ` [PATCH v2 1/2] dmaengine: idxd: Set wq state to disabled in idxd_wq_disable_cleanup() Jerry Snitselaar
@ 2022-09-28 15:54 ` Dave Jiang
0 siblings, 0 replies; 6+ messages in thread
From: Dave Jiang @ 2022-09-28 15:54 UTC (permalink / raw)
To: Jerry Snitselaar, linux-kernel, dmaengine; +Cc: Fenghua Yu, Vinod Koul
On 9/28/2022 8:48 AM, Jerry Snitselaar wrote:
> 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 <fenghua.yu@intel.com>
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Vinod Koul <vkoul@kernel.org>
> Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> 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;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/2] dmaengine: idxd: Fix up re-enabling device workqueues
2022-09-28 15:48 [PATCH v2 0/2] dmaengine: idxd: Fix up re-enabling device workqueues Jerry Snitselaar
2022-09-28 15:48 ` [PATCH v2 1/2] dmaengine: idxd: Set wq state to disabled in idxd_wq_disable_cleanup() Jerry Snitselaar
2022-09-28 15:48 ` [PATCH v2 2/2] dmaengine: idxd: track enabled workqueues in bitmap Jerry Snitselaar
@ 2022-09-29 7:32 ` Vinod Koul
2 siblings, 0 replies; 6+ messages in thread
From: Vinod Koul @ 2022-09-29 7:32 UTC (permalink / raw)
To: Jerry Snitselaar; +Cc: linux-kernel, dmaengine, Fenghua Yu, Dave Jiang
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-29 7:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-28 15:48 [PATCH v2 0/2] dmaengine: idxd: Fix up re-enabling device workqueues Jerry Snitselaar
2022-09-28 15:48 ` [PATCH v2 1/2] dmaengine: idxd: Set wq state to disabled in idxd_wq_disable_cleanup() Jerry Snitselaar
2022-09-28 15:54 ` Dave Jiang
2022-09-28 15:48 ` [PATCH v2 2/2] dmaengine: idxd: track enabled workqueues in bitmap Jerry Snitselaar
2022-09-28 15:51 ` Dave Jiang
2022-09-29 7:32 ` [PATCH v2 0/2] dmaengine: idxd: Fix up re-enabling device workqueues Vinod Koul
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox