Re: [PATCH 7/9] s390: Convert from tasklet to BH workqueue
On 2024-03-27 17:03, Allen Pais wrote: The only generic interface to execute asynchronously in the BH context is tasklet; however, it's marked deprecated and has some design flaws. To replace tasklets, BH workqueue support was recently added. A BH workqueue behaves similarly to regular workqueues except that the queued work items are executed in the BH context. This patch converts drivers/infiniband/* from tasklet to BH workqueue. Based on the work done by Tejun Heo Branch: https://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-6.10 Note: Not tested. Please test/review. Signed-off-by: Allen Pais --- ... drivers/s390/crypto/ap_bus.c | 24 +++--- drivers/s390/crypto/ap_bus.h | 2 +- drivers/s390/crypto/zcrypt_msgtype50.c | 2 +- drivers/s390/crypto/zcrypt_msgtype6.c | 4 +-- ... Applied and tested the s390 AP bus and zcrypt part of the patch. Works fine, a sniff test did not show any problems. Thanks for your work. Reviewed-by: Harald Freudenberger
Re: [PATCH 7/9] s390: Convert from tasklet to BH workqueue
On Wed, Mar 27, 2024 at 04:03:12PM +, Allen Pais wrote:
> The only generic interface to execute asynchronously in the BH context is
> tasklet; however, it's marked deprecated and has some design flaws. To
> replace tasklets, BH workqueue support was recently added. A BH workqueue
> behaves similarly to regular workqueues except that the queued work items
> are executed in the BH context.
>
> This patch converts drivers/infiniband/* from tasklet to BH workqueue.
>
> Based on the work done by Tejun Heo
> Branch: https://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-6.10
I guess this dependency is a hard requirement due to commit 134874e2eee9
("workqueue: Allow cancel_work_sync() and disable_work() from atomic contexts
on BH work items")?
> ---
> drivers/s390/block/dasd.c | 42
> drivers/s390/block/dasd_int.h | 10 +++---
> drivers/s390/char/con3270.c| 27
> drivers/s390/crypto/ap_bus.c | 24 +++---
> drivers/s390/crypto/ap_bus.h | 2 +-
> drivers/s390/crypto/zcrypt_msgtype50.c | 2 +-
> drivers/s390/crypto/zcrypt_msgtype6.c | 4 +--
> drivers/s390/net/ctcm_fsms.c | 4 +--
> drivers/s390/net/ctcm_main.c | 15 -
> drivers/s390/net/ctcm_main.h | 5 +--
> drivers/s390/net/ctcm_mpc.c| 12 +++
> drivers/s390/net/ctcm_mpc.h| 7 ++--
> drivers/s390/net/lcs.c | 26 +++
> drivers/s390/net/lcs.h | 2 +-
> drivers/s390/net/qeth_core_main.c | 2 +-
> drivers/s390/scsi/zfcp_qdio.c | 45 +-
> drivers/s390/scsi/zfcp_qdio.h | 9 +++---
> 17 files changed, 117 insertions(+), 121 deletions(-)
I'm asking since this patch comes with multiple compile errors. Probably due
to lack of cross compiler tool chain on your side.
If the above wouldn't be a hard dependency I'd say we could take those parts
of your patch which are fine into the s390 tree for 6.10, fix the rest, and
schedule that as well for 6.10 via the s390 tree.
Re: [PATCH 7/9] s390: Convert from tasklet to BH workqueue
> > > > Signed-off-by: Allen Pais > > --- > > drivers/s390/block/dasd.c | 42 > > drivers/s390/block/dasd_int.h | 10 +++--- > > drivers/s390/char/con3270.c| 27 > > drivers/s390/crypto/ap_bus.c | 24 +++--- > > drivers/s390/crypto/ap_bus.h | 2 +- > > drivers/s390/crypto/zcrypt_msgtype50.c | 2 +- > > drivers/s390/crypto/zcrypt_msgtype6.c | 4 +-- > > drivers/s390/net/ctcm_fsms.c | 4 +-- > > drivers/s390/net/ctcm_main.c | 15 - > > drivers/s390/net/ctcm_main.h | 5 +-- > > drivers/s390/net/ctcm_mpc.c| 12 +++ > > drivers/s390/net/ctcm_mpc.h| 7 ++-- > > drivers/s390/net/lcs.c | 26 +++ > > drivers/s390/net/lcs.h | 2 +- > > drivers/s390/net/qeth_core_main.c | 2 +- > > drivers/s390/scsi/zfcp_qdio.c | 45 +- > > drivers/s390/scsi/zfcp_qdio.h | 9 +++--- > > 17 files changed, 117 insertions(+), 121 deletions(-) > > > > > We're looking into the best way to test this. > > For drivers/s390/net/ctcm* and drivers/s390/net/lcs*: > Acked-by: Alexandra Winter Thank you very much. > > > [...] > > diff --git a/drivers/s390/net/qeth_core_main.c > > b/drivers/s390/net/qeth_core_main.c > > index a0cce6872075..10ea95abc753 100644 > > --- a/drivers/s390/net/qeth_core_main.c > > +++ b/drivers/s390/net/qeth_core_main.c > > @@ -2911,7 +2911,7 @@ static int qeth_init_input_buffer(struct qeth_card > > *card, > > } > > > > /* > > - * since the buffer is accessed only from the input_tasklet > > + * since the buffer is accessed only from the input_work > >* there shouldn't be a need to synchronize; also, since we use > >* the QETH_IN_BUF_REQUEUE_THRESHOLD we should never run out off > >* buffers > > I propose to delete the whole comment block. There have been many changes and > I don't think it is helpful for the current qeth driver. Sure, I will have it fixed in v2. - Allen
Re: [PATCH 7/9] s390: Convert from tasklet to BH workqueue
On 27.03.24 17:03, Allen Pais wrote: > The only generic interface to execute asynchronously in the BH context is > tasklet; however, it's marked deprecated and has some design flaws. To > replace tasklets, BH workqueue support was recently added. A BH workqueue > behaves similarly to regular workqueues except that the queued work items > are executed in the BH context. > > This patch converts drivers/infiniband/* from tasklet to BH workqueue. > > Based on the work done by Tejun Heo > Branch: https://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-6.10 > > Note: Not tested. Please test/review. > > Signed-off-by: Allen Pais > --- > drivers/s390/block/dasd.c | 42 > drivers/s390/block/dasd_int.h | 10 +++--- > drivers/s390/char/con3270.c| 27 > drivers/s390/crypto/ap_bus.c | 24 +++--- > drivers/s390/crypto/ap_bus.h | 2 +- > drivers/s390/crypto/zcrypt_msgtype50.c | 2 +- > drivers/s390/crypto/zcrypt_msgtype6.c | 4 +-- > drivers/s390/net/ctcm_fsms.c | 4 +-- > drivers/s390/net/ctcm_main.c | 15 - > drivers/s390/net/ctcm_main.h | 5 +-- > drivers/s390/net/ctcm_mpc.c| 12 +++ > drivers/s390/net/ctcm_mpc.h| 7 ++-- > drivers/s390/net/lcs.c | 26 +++ > drivers/s390/net/lcs.h | 2 +- > drivers/s390/net/qeth_core_main.c | 2 +- > drivers/s390/scsi/zfcp_qdio.c | 45 +- > drivers/s390/scsi/zfcp_qdio.h | 9 +++--- > 17 files changed, 117 insertions(+), 121 deletions(-) > We're looking into the best way to test this. For drivers/s390/net/ctcm* and drivers/s390/net/lcs*: Acked-by: Alexandra Winter [...] > diff --git a/drivers/s390/net/qeth_core_main.c > b/drivers/s390/net/qeth_core_main.c > index a0cce6872075..10ea95abc753 100644 > --- a/drivers/s390/net/qeth_core_main.c > +++ b/drivers/s390/net/qeth_core_main.c > @@ -2911,7 +2911,7 @@ static int qeth_init_input_buffer(struct qeth_card > *card, > } > > /* > - * since the buffer is accessed only from the input_tasklet > + * since the buffer is accessed only from the input_work >* there shouldn't be a need to synchronize; also, since we use >* the QETH_IN_BUF_REQUEUE_THRESHOLD we should never run out off >* buffers I propose to delete the whole comment block. There have been many changes and I don't think it is helpful for the current qeth driver.
[PATCH 7/9] s390: Convert from tasklet to BH workqueue
The only generic interface to execute asynchronously in the BH context is
tasklet; however, it's marked deprecated and has some design flaws. To
replace tasklets, BH workqueue support was recently added. A BH workqueue
behaves similarly to regular workqueues except that the queued work items
are executed in the BH context.
This patch converts drivers/infiniband/* from tasklet to BH workqueue.
Based on the work done by Tejun Heo
Branch: https://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-6.10
Note: Not tested. Please test/review.
Signed-off-by: Allen Pais
---
drivers/s390/block/dasd.c | 42
drivers/s390/block/dasd_int.h | 10 +++---
drivers/s390/char/con3270.c| 27
drivers/s390/crypto/ap_bus.c | 24 +++---
drivers/s390/crypto/ap_bus.h | 2 +-
drivers/s390/crypto/zcrypt_msgtype50.c | 2 +-
drivers/s390/crypto/zcrypt_msgtype6.c | 4 +--
drivers/s390/net/ctcm_fsms.c | 4 +--
drivers/s390/net/ctcm_main.c | 15 -
drivers/s390/net/ctcm_main.h | 5 +--
drivers/s390/net/ctcm_mpc.c| 12 +++
drivers/s390/net/ctcm_mpc.h| 7 ++--
drivers/s390/net/lcs.c | 26 +++
drivers/s390/net/lcs.h | 2 +-
drivers/s390/net/qeth_core_main.c | 2 +-
drivers/s390/scsi/zfcp_qdio.c | 45 +-
drivers/s390/scsi/zfcp_qdio.h | 9 +++---
17 files changed, 117 insertions(+), 121 deletions(-)
diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c
index 0a97cfedd706..c6f9910f0a98 100644
--- a/drivers/s390/block/dasd.c
+++ b/drivers/s390/block/dasd.c
@@ -54,8 +54,8 @@ MODULE_LICENSE("GPL");
* SECTION: prototypes for static functions of dasd.c
*/
static int dasd_flush_block_queue(struct dasd_block *);
-static void dasd_device_tasklet(unsigned long);
-static void dasd_block_tasklet(unsigned long);
+static void dasd_device_work(struct work_struct *);
+static void dasd_block_work(struct work_struct *);
static void do_kick_device(struct work_struct *);
static void do_reload_device(struct work_struct *);
static void do_requeue_requests(struct work_struct *);
@@ -114,9 +114,8 @@ struct dasd_device *dasd_alloc_device(void)
dasd_init_chunklist(>erp_chunks, device->erp_mem, PAGE_SIZE);
dasd_init_chunklist(>ese_chunks, device->ese_mem, PAGE_SIZE *
2);
spin_lock_init(>mem_lock);
- atomic_set(>tasklet_scheduled, 0);
- tasklet_init(>tasklet, dasd_device_tasklet,
-(unsigned long) device);
+ atomic_set(>work_scheduled, 0);
+ INIT_WORK(>bh, dasd_device_work);
INIT_LIST_HEAD(>ccw_queue);
timer_setup(>timer, dasd_device_timeout, 0);
INIT_WORK(>kick_work, do_kick_device);
@@ -154,9 +153,8 @@ struct dasd_block *dasd_alloc_block(void)
/* open_count = 0 means device online but not in use */
atomic_set(>open_count, -1);
- atomic_set(>tasklet_scheduled, 0);
- tasklet_init(>tasklet, dasd_block_tasklet,
-(unsigned long) block);
+ atomic_set(>work_scheduled, 0);
+ INIT_WORK(>bh, dasd_block_work);
INIT_LIST_HEAD(>ccw_queue);
spin_lock_init(>queue_lock);
INIT_LIST_HEAD(>format_list);
@@ -2148,12 +2146,12 @@ EXPORT_SYMBOL_GPL(dasd_flush_device_queue);
/*
* Acquire the device lock and process queues for the device.
*/
-static void dasd_device_tasklet(unsigned long data)
+static void dasd_device_work(struct work_struct *t)
{
- struct dasd_device *device = (struct dasd_device *) data;
+ struct dasd_device *device = from_work(device, t, bh);
struct list_head final_queue;
- atomic_set (>tasklet_scheduled, 0);
+ atomic_set (>work_scheduled, 0);
INIT_LIST_HEAD(_queue);
spin_lock_irq(get_ccwdev_lock(device->cdev));
/* Check expire time of first request on the ccw queue. */
@@ -2174,15 +2172,15 @@ static void dasd_device_tasklet(unsigned long data)
}
/*
- * Schedules a call to dasd_tasklet over the device tasklet.
+ * Schedules a call to dasd_work over the device wq.
*/
void dasd_schedule_device_bh(struct dasd_device *device)
{
/* Protect against rescheduling. */
- if (atomic_cmpxchg (>tasklet_scheduled, 0, 1) != 0)
+ if (atomic_cmpxchg (>work_scheduled, 0, 1) != 0)
return;
dasd_get_device(device);
- tasklet_hi_schedule(>tasklet);
+ queue_work(system_bh_highpri_wq, >bh);
}
EXPORT_SYMBOL(dasd_schedule_device_bh);
@@ -2595,7 +2593,7 @@ int dasd_sleep_on_immediatly(struct dasd_ccw_req *cqr)
else
rc = -EIO;
- /* kick tasklets */
+ /* kick works */
dasd_schedule_device_bh(device);
if (device->block)
dasd_schedule_block_bh(device->block);
@@ -2891,15 +2889,15 @@ static void __dasd_block_start_head(struct dasd_block
