lkml.org 
[lkml]   [2006]   [Jul]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: please revert kthread from loop.c
> But not good for me.  Gets further e.g. 170 iterations,
> but then hangs while kthread_stop waits for completion.

After getting much more familiar with the code, here is a more invasive,
but pretty heavily tested patch.

> I haven't investigated further. Is there really any reason
> to be messing with what has worked well for so long here?

If the EXPORT_SYMBOL(kernel_thread) is going to be removed, then
this code will have to be updated. Otherwise, no, the code as is
doesn't use pids to reference tasks, so the original code would be
fine with me.

thanks,
-serge

From: Serge Hallyn <serue@us.ibm.com>
Subject: [PATCH] kthread: convert loop.c to kthread

Convert loop.c from the deprecated kernel_thread to kthread.
This patch also simplifies the code a bit. It has passed
vigerous testing. The following script, looptorture.sh,
which mounts and unmounts loopback device 1000 times,
completes in three terminals simultaneously on different
loop devices. For example, started up screen, created
three screens, and typed each of the following lines into
a separate screen:
sh loopback.sh 2
sh loopback.sh 3
sh loopback.sh 4

loopback.sh:
j=0
if [ $# > 0 ]; then
num=$1
else
num="0"
fi
echo "starting loop ($num)"
if [ ! -d /mnt/$num ]; then
mkdir /mnt/$num
fi
dd if=/dev/zero of=/tst/zero$num bs=1M count=100
while [ $j -lt 1000 ]; do
let j=j+1
echo "Doing pass $j"
losetup /dev/loop$num /tst/zero$num
mkfs -t ext2 -b 1024 /dev/loop$num >/dev/null 2>&1
mount -t ext2 /dev/loop$num /mnt/$num
echo hello > /mnt/$num/hw
umount /mnt/$num
losetup -d /dev/loop$num
done

A partial kernel build on a loopback partition also worked fine.
I have not tested encrypted devices, or suspend to loopback device.

Changes since last attempt:
Eliminated lo->lo_done and lo->lo_bh_done completions.
The former is not needed because we can wake up the thread
when we're ready. The latter was used for waking the
loop_thread up when there was something to do or it was
time to quit, and we no longer need this since we simply
wake the thread directly when there's data.

The lo->pending count used to be set at one plus actual
pending actions, and was reduced to 0 only when it was time
to quit. We now use kthread_should_stop() to find out whether
we're done, though we do not actually stop until there are no
pending actions.

Signed-off-by: Serge Hallyn <serue@us.ibm.com>

---

drivers/block/loop.c | 71 ++++++++++++++++++--------------------------------
include/linux/loop.h | 3 +-
2 files changed, 27 insertions(+), 47 deletions(-)

b56590212f3050dbb6c630eeea3a44794c9c7439
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index ccaada1..bdad531 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -72,6 +72,7 @@
#include <linux/completion.h>
#include <linux/highmem.h>
#include <linux/gfp.h>
+#include <linux/kthread.h>

#include <asm/uaccess.h>

@@ -525,12 +526,10 @@ static int loop_make_request(request_que
lo->lo_pending++;
loop_add_bio(lo, old_bio);
spin_unlock_irq(&lo->lo_lock);
- complete(&lo->lo_bh_done);
+ wake_up_process(lo->lo_thread);
return 0;

out:
- if (lo->lo_pending == 0)
- complete(&lo->lo_bh_done);
spin_unlock_irq(&lo->lo_lock);
bio_io_error(old_bio, old_bio->bi_size);
return 0;
@@ -576,8 +575,6 @@ static int loop_thread(void *data)
struct loop_device *lo = data;
struct bio *bio;

- daemonize("loop%d", lo->lo_number);
-
/*
* loop can be used in an encrypted device,
* hence, it mustn't be stopped at all
@@ -587,47 +584,28 @@ static int loop_thread(void *data)

set_user_nice(current, -20);

- lo->lo_state = Lo_bound;
- lo->lo_pending = 1;
-
- /*
- * complete it, we are running
- */
- complete(&lo->lo_done);
-
for (;;) {
- int pending;
-
- if (wait_for_completion_interruptible(&lo->lo_bh_done))
- continue;
-
spin_lock_irq(&lo->lo_lock);
+ while (lo->lo_pending) {
+ bio = loop_get_bio(lo);
+ lo->lo_pending--;

- /*
- * could be completed because of tear-down, not pending work
- */
- if (unlikely(!lo->lo_pending)) {
spin_unlock_irq(&lo->lo_lock);
- break;
+ BUG_ON(!bio);
+ loop_handle_bio(lo, bio);
+ spin_lock_irq(&lo->lo_lock);
}

- bio = loop_get_bio(lo);
- lo->lo_pending--;
- pending = lo->lo_pending;
+ if (kthread_should_stop()) {
+ spin_unlock_irq(&lo->lo_lock);
+ break;
+ }
spin_unlock_irq(&lo->lo_lock);

- BUG_ON(!bio);
- loop_handle_bio(lo, bio);
-
- /*
- * upped both for pending work and tear-down, lo_pending
- * will hit zero then
- */
- if (unlikely(!pending))
- break;
+ __set_current_state(TASK_INTERRUPTIBLE);
+ schedule();
}

- complete(&lo->lo_done);
return 0;
}

@@ -846,10 +824,16 @@ static int loop_set_fd(struct loop_devic

set_blocksize(bdev, lo_blocksize);

- error = kernel_thread(loop_thread, lo, CLONE_KERNEL);
- if (error < 0)
+ lo->lo_thread = kthread_create(loop_thread, lo, "loop%d",
+ lo->lo_number);
+ if (IS_ERR(lo->lo_thread)) {
+ error = PTR_ERR(lo->lo_thread);
+ lo->lo_thread = NULL;
goto out_putf;
- wait_for_completion(&lo->lo_done);
+ }
+ lo->lo_pending = 0;
+ lo->lo_state = Lo_bound;
+ wake_up_process(lo->lo_thread);
return 0;

out_putf:
@@ -913,12 +897,9 @@ static int loop_clr_fd(struct loop_devic

spin_lock_irq(&lo->lo_lock);
lo->lo_state = Lo_rundown;
- lo->lo_pending--;
- if (!lo->lo_pending)
- complete(&lo->lo_bh_done);
spin_unlock_irq(&lo->lo_lock);

- wait_for_completion(&lo->lo_done);
+ kthread_stop(lo->lo_thread);

lo->lo_backing_file = NULL;

@@ -928,6 +909,7 @@ static int loop_clr_fd(struct loop_devic
lo->lo_device = NULL;
lo->lo_encryption = NULL;
lo->lo_offset = 0;
+ lo->lo_thread = NULL;
lo->lo_sizelimit = 0;
lo->lo_encrypt_key_size = 0;
lo->lo_flags = 0;
@@ -1293,8 +1275,7 @@ static int __init loop_init(void)
if (!lo->lo_queue)
goto out_mem4;
mutex_init(&lo->lo_ctl_mutex);
- init_completion(&lo->lo_done);
- init_completion(&lo->lo_bh_done);
+ lo->lo_thread = NULL;
lo->lo_number = i;
spin_lock_init(&lo->lo_lock);
disk->major = LOOP_MAJOR;
diff --git a/include/linux/loop.h b/include/linux/loop.h
index e76c761..51dee29 100644
--- a/include/linux/loop.h
+++ b/include/linux/loop.h
@@ -59,8 +59,7 @@ struct loop_device {
struct bio *lo_bio;
struct bio *lo_biotail;
int lo_state;
- struct completion lo_done;
- struct completion lo_bh_done;
+ struct task_struct *lo_thread;
struct mutex lo_ctl_mutex;
int lo_pending;

--
1.1.6
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2006-07-11 21:53    [W:0.116 / U:0.340 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site