Re: [PATCH 1/2] block: loop: convert to per-device workqueue
On 05/05/2015 06:49 AM, Ming Lei wrote:
> Documentation/workqueue.txt:
> If there is dependency among multiple work items used
> during memory reclaim, they should be queued to separate
> wq each with WQ_MEM_RECLAIM.
>
> Loop devices can be stacked, so we have to convert to per-device
> workqueue. One example is Fedora live CD.
I'm just now looking at this, but I found a minor nit. I'm not sure if
it's worth replacing the patch or just fixing it with a trivial cleanup
patch.
>
> Fixes: b5dd2f6047ca108001328aac0e8588edd15f1778
> Cc: sta...@vger.kernel.org (v4.0)
> Cc: Justin M. Forbes
> Signed-off-by: Ming Lei
> ---
> drivers/block/loop.c | 30 ++
> drivers/block/loop.h | 1 +
> 2 files changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index ae3fcb4..3dc1598 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -86,8 +86,6 @@ static DEFINE_MUTEX(loop_index_mutex);
> static int max_part;
> static int part_shift;
>
> -static struct workqueue_struct *loop_wq;
> -
> static int transfer_xor(struct loop_device *lo, int cmd,
> struct page *raw_page, unsigned raw_off,
> struct page *loop_page, unsigned loop_off,
> @@ -725,6 +723,12 @@ static int loop_set_fd(struct loop_device *lo, fmode_t
> mode,
> size = get_loop_size(lo, file);
> if ((loff_t)(sector_t)size != size)
> goto out_putf;
> + error = -ENOMEM;
> + lo->wq = alloc_workqueue("kloopd%d",
> + WQ_MEM_RECLAIM | WQ_HIGHPRI | WQ_UNBOUND, 0,
> + lo->lo_number);
> + if (!lo->wq)
> + goto out_putf;
>
> error = 0;
>
> @@ -872,6 +876,8 @@ static int loop_clr_fd(struct loop_device *lo)
> lo->lo_flags = 0;
> if (!part_shift)
> lo->lo_disk->flags |= GENHD_FL_NO_PART_SCAN;
> + destroy_workqueue(lo->wq);
> + lo->wq = NULL;
> mutex_unlock(&lo->lo_ctl_mutex);
> /*
>* Need not hold lo_ctl_mutex to fput backing file.
> @@ -1425,9 +1431,13 @@ static int loop_queue_rq(struct blk_mq_hw_ctx *hctx,
> const struct blk_mq_queue_data *bd)
> {
> struct loop_cmd *cmd = blk_mq_rq_to_pdu(bd->rq);
> + struct loop_device *lo = cmd->rq->q->queuedata;
>
> blk_mq_start_request(bd->rq);
>
> + if (lo->lo_state != Lo_bound)
> + return -EIO;
> +
> if (cmd->rq->cmd_flags & REQ_WRITE) {
> struct loop_device *lo = cmd->rq->q->queuedata;
The above definition of lo becomes redundant now.
> bool need_sched = true;
> @@ -1441,9 +1451,9 @@ static int loop_queue_rq(struct blk_mq_hw_ctx *hctx,
> spin_unlock_irq(&lo->lo_lock);
>
> if (need_sched)
> - queue_work(loop_wq, &lo->write_work);
> + queue_work(lo->wq, &lo->write_work);
> } else {
> - queue_work(loop_wq, &cmd->read_work);
> + queue_work(lo->wq, &cmd->read_work);
> }
>
> return BLK_MQ_RQ_QUEUE_OK;
> @@ -1455,9 +1465,6 @@ static void loop_handle_cmd(struct loop_cmd *cmd)
> struct loop_device *lo = cmd->rq->q->queuedata;
> int ret = -EIO;
>
> - if (lo->lo_state != Lo_bound)
> - goto failed;
> -
> if (write && (lo->lo_flags & LO_FLAGS_READ_ONLY))
> goto failed;
>
> @@ -1806,13 +1813,6 @@ static int __init loop_init(void)
> goto misc_out;
> }
>
> - loop_wq = alloc_workqueue("kloopd",
> - WQ_MEM_RECLAIM | WQ_HIGHPRI | WQ_UNBOUND, 0);
> - if (!loop_wq) {
> - err = -ENOMEM;
> - goto misc_out;
> - }
> -
> blk_register_region(MKDEV(LOOP_MAJOR, 0), range,
> THIS_MODULE, loop_probe, NULL, NULL);
>
> @@ -1850,8 +1850,6 @@ static void __exit loop_exit(void)
> blk_unregister_region(MKDEV(LOOP_MAJOR, 0), range);
> unregister_blkdev(LOOP_MAJOR, "loop");
>
> - destroy_workqueue(loop_wq);
> -
> misc_deregister(&loop_misc);
> }
>
> diff --git a/drivers/block/loop.h b/drivers/block/loop.h
> index 301c27f..49564ed 100644
> --- a/drivers/block/loop.h
> +++ b/drivers/block/loop.h
> @@ -54,6 +54,7 @@ struct loop_device {
> gfp_t old_gfp_mask;
>
> spinlock_t lo_lock;
> + struct workqueue_struct *wq;
> struct list_headwrite_cmd_head;
> struct work_struct write_work;
> boolwrite_started;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] block: loop: convert to per-device workqueue
On Tue, May 05, 2015 at 07:49:54PM +0800, Ming Lei wrote: > Documentation/workqueue.txt: > If there is dependency among multiple work items used > during memory reclaim, they should be queued to separate > wq each with WQ_MEM_RECLAIM. > > Loop devices can be stacked, so we have to convert to per-device > workqueue. One example is Fedora live CD. > > Fixes: b5dd2f6047ca108001328aac0e8588edd15f1778 > Cc: sta...@vger.kernel.org (v4.0) > Cc: Justin M. Forbes > Signed-off-by: Ming Lei Acked-by: Tejun Heo Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] block: loop: convert to per-device workqueue
Documentation/workqueue.txt:
If there is dependency among multiple work items used
during memory reclaim, they should be queued to separate
wq each with WQ_MEM_RECLAIM.
Loop devices can be stacked, so we have to convert to per-device
workqueue. One example is Fedora live CD.
Fixes: b5dd2f6047ca108001328aac0e8588edd15f1778
Cc: sta...@vger.kernel.org (v4.0)
Cc: Justin M. Forbes
Signed-off-by: Ming Lei
---
drivers/block/loop.c | 30 ++
drivers/block/loop.h | 1 +
2 files changed, 15 insertions(+), 16 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index ae3fcb4..3dc1598 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -86,8 +86,6 @@ static DEFINE_MUTEX(loop_index_mutex);
static int max_part;
static int part_shift;
-static struct workqueue_struct *loop_wq;
-
static int transfer_xor(struct loop_device *lo, int cmd,
struct page *raw_page, unsigned raw_off,
struct page *loop_page, unsigned loop_off,
@@ -725,6 +723,12 @@ static int loop_set_fd(struct loop_device *lo, fmode_t
mode,
size = get_loop_size(lo, file);
if ((loff_t)(sector_t)size != size)
goto out_putf;
+ error = -ENOMEM;
+ lo->wq = alloc_workqueue("kloopd%d",
+ WQ_MEM_RECLAIM | WQ_HIGHPRI | WQ_UNBOUND, 0,
+ lo->lo_number);
+ if (!lo->wq)
+ goto out_putf;
error = 0;
@@ -872,6 +876,8 @@ static int loop_clr_fd(struct loop_device *lo)
lo->lo_flags = 0;
if (!part_shift)
lo->lo_disk->flags |= GENHD_FL_NO_PART_SCAN;
+ destroy_workqueue(lo->wq);
+ lo->wq = NULL;
mutex_unlock(&lo->lo_ctl_mutex);
/*
* Need not hold lo_ctl_mutex to fput backing file.
@@ -1425,9 +1431,13 @@ static int loop_queue_rq(struct blk_mq_hw_ctx *hctx,
const struct blk_mq_queue_data *bd)
{
struct loop_cmd *cmd = blk_mq_rq_to_pdu(bd->rq);
+ struct loop_device *lo = cmd->rq->q->queuedata;
blk_mq_start_request(bd->rq);
+ if (lo->lo_state != Lo_bound)
+ return -EIO;
+
if (cmd->rq->cmd_flags & REQ_WRITE) {
struct loop_device *lo = cmd->rq->q->queuedata;
bool need_sched = true;
@@ -1441,9 +1451,9 @@ static int loop_queue_rq(struct blk_mq_hw_ctx *hctx,
spin_unlock_irq(&lo->lo_lock);
if (need_sched)
- queue_work(loop_wq, &lo->write_work);
+ queue_work(lo->wq, &lo->write_work);
} else {
- queue_work(loop_wq, &cmd->read_work);
+ queue_work(lo->wq, &cmd->read_work);
}
return BLK_MQ_RQ_QUEUE_OK;
@@ -1455,9 +1465,6 @@ static void loop_handle_cmd(struct loop_cmd *cmd)
struct loop_device *lo = cmd->rq->q->queuedata;
int ret = -EIO;
- if (lo->lo_state != Lo_bound)
- goto failed;
-
if (write && (lo->lo_flags & LO_FLAGS_READ_ONLY))
goto failed;
@@ -1806,13 +1813,6 @@ static int __init loop_init(void)
goto misc_out;
}
- loop_wq = alloc_workqueue("kloopd",
- WQ_MEM_RECLAIM | WQ_HIGHPRI | WQ_UNBOUND, 0);
- if (!loop_wq) {
- err = -ENOMEM;
- goto misc_out;
- }
-
blk_register_region(MKDEV(LOOP_MAJOR, 0), range,
THIS_MODULE, loop_probe, NULL, NULL);
@@ -1850,8 +1850,6 @@ static void __exit loop_exit(void)
blk_unregister_region(MKDEV(LOOP_MAJOR, 0), range);
unregister_blkdev(LOOP_MAJOR, "loop");
- destroy_workqueue(loop_wq);
-
misc_deregister(&loop_misc);
}
diff --git a/drivers/block/loop.h b/drivers/block/loop.h
index 301c27f..49564ed 100644
--- a/drivers/block/loop.h
+++ b/drivers/block/loop.h
@@ -54,6 +54,7 @@ struct loop_device {
gfp_t old_gfp_mask;
spinlock_t lo_lock;
+ struct workqueue_struct *wq;
struct list_headwrite_cmd_head;
struct work_struct write_work;
boolwrite_started;
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
