Update the s390 cooperative memory manager, which can be a module,
to use kthread rather than kernel_thread, whose EXPORT is deprecated.
This patch compiles and boots fine, but I don't know how to really
test the driver.
Signed-off-by: Serge E. Hallyn <[email protected]>
---
arch/s390/mm/cmm.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
7f73a7a8a72647c0bd08ba5c47e941ddf72badee
diff --git a/arch/s390/mm/cmm.c b/arch/s390/mm/cmm.c
index ceea51c..a4d463d 100644
--- a/arch/s390/mm/cmm.c
+++ b/arch/s390/mm/cmm.c
@@ -15,6 +15,7 @@
#include <linux/sched.h>
#include <linux/sysctl.h>
#include <linux/ctype.h>
+#include <linux/kthread.h>
#include <asm/pgalloc.h>
#include <asm/uaccess.h>
@@ -126,7 +127,6 @@ cmm_thread(void *dummy)
{
int rc;
- daemonize("cmmthread");
while (1) {
rc = wait_event_interruptible(cmm_thread_wait,
(cmm_pages != cmm_pages_target ||
@@ -161,7 +161,7 @@ cmm_thread(void *dummy)
static void
cmm_start_thread(void)
{
- kernel_thread(cmm_thread, NULL, 0);
+ kthread_run(cmm_thread, NULL, "cmmthread");
}
static void
--
1.1.6
On Thu, 2006-08-24 at 16:22 -0500, Serge E. Hallyn wrote:
> This patch compiles and boots fine, but I don't know how to really
> test the driver.
Just tried the patch and tested cmm. Still works fine. Patch added to my
"things-that-will-go-out-after-2.6.18" list of patches.
--
blue skies,
Martin.
Martin Schwidefsky
Linux for zSeries Development & Services
IBM Deutschland Entwicklung GmbH
"Reality continues to ruin my life." - Calvin.
On Fri, 2006-08-25 at 10:53 +0200, Martin Schwidefsky wrote:
> > This patch compiles and boots fine, but I don't know how to really
> > test the driver.
>
> Just tried the patch and tested cmm. Still works fine. Patch added to my
> "things-that-will-go-out-after-2.6.18" list of patches.
Heiko just pointed out that this has already been fixed. His patch
depends on another patch (oom-killer) and can be found in the current
-mm tree. I'll drop this patch.
--
blue skies,
Martin.
Martin Schwidefsky
Linux for zSeries Development & Services
IBM Deutschland Entwicklung GmbH
"Reality continues to ruin my life." - Calvin.
On Thu, Aug 24, 2006 at 04:22:42PM -0500, Serge E. Hallyn wrote:
> Update the s390 cooperative memory manager, which can be a module,
> to use kthread rather than kernel_thread, whose EXPORT is deprecated.
>
> This patch compiles and boots fine, but I don't know how to really
> test the driver.
NACK. Please do a real conversion to the kthread paradigm instead of
doctoring around the trivial bits that could be changed with a script.
Please use kthread_should_stop() and remove the cmm_thread_wait
waitqueue in favour of wake_up_process. The timer useage could
probably be replaced with smart usage of schedule_timeout().
Also the code seems to miss a proper thread termination on module
removal.
Quoting Martin Schwidefsky ([email protected]):
> On Fri, 2006-08-25 at 10:53 +0200, Martin Schwidefsky wrote:
> > > This patch compiles and boots fine, but I don't know how to really
> > > test the driver.
> >
> > Just tried the patch and tested cmm. Still works fine. Patch added to my
> > "things-that-will-go-out-after-2.6.18" list of patches.
>
> Heiko just pointed out that this has already been fixed. His patch
> depends on another patch (oom-killer) and can be found in the current
> -mm tree. I'll drop this patch.
Great, thanks.
Do you know whether anyone is working on doing a proper update for the
other two patches I submitted, as per Christoph's comments? I'd be
timit about gutting those drivers to that extent on my own...
thanks,
-serge
Quoting Christoph Hellwig ([email protected]):
> On Thu, Aug 24, 2006 at 04:22:42PM -0500, Serge E. Hallyn wrote:
> > Update the s390 cooperative memory manager, which can be a module,
> > to use kthread rather than kernel_thread, whose EXPORT is deprecated.
> >
> > This patch compiles and boots fine, but I don't know how to really
> > test the driver.
>
> NACK. Please do a real conversion to the kthread paradigm instead of
> doctoring around the trivial bits that could be changed with a script.
>
> Please use kthread_should_stop() and remove the cmm_thread_wait
> waitqueue in favour of wake_up_process. The timer useage could
> probably be replaced with smart usage of schedule_timeout().
> Also the code seems to miss a proper thread termination on module
> removal.
Ok, the patch in -mm does kthread_stop() on module_exit, but still uses
the timer and cmm_thread_wait.
I'm not clear what the timer is actually trying to do, or why there is a
separate cmm_pages_target and cmm_timed_pages_target. So I'm sure the
below patch on top of -mm2 is wrong (it compiles, but I just noticed
2.6.18-rc4-mm2 doesn't boot without this patch either) but hopefully
Heiko or Martin can tell me what would be the right way, or implement
it?
thanks,
-serge
Subject: [PATCH] s390: stop using cmm_thread_wait and cmm_timer in cmm
Update cmm to stop using cmm_thread_wait or cmm_timer.
Signed-off-by: Serge E. Hallyn <[email protected]>
---
arch/s390/mm/cmm.c | 67 +++++++++-------------------------------------------
1 files changed, 12 insertions(+), 55 deletions(-)
4182dbd937e5084db4cfd63193ff93267ea8042e
diff --git a/arch/s390/mm/cmm.c b/arch/s390/mm/cmm.c
index 9b62157..ecd237a 100644
--- a/arch/s390/mm/cmm.c
+++ b/arch/s390/mm/cmm.c
@@ -48,11 +48,6 @@ static struct cmm_page_array *cmm_timed_
static DEFINE_SPINLOCK(cmm_lock);
static struct task_struct *cmm_thread_ptr;
-static wait_queue_head_t cmm_thread_wait;
-static struct timer_list cmm_timer;
-
-static void cmm_timer_fn(unsigned long);
-static void cmm_set_timer(void);
static long
cmm_strtoul(const char *cp, char **endp)
@@ -157,18 +152,10 @@ static struct notifier_block cmm_oom_nb
static int
cmm_thread(void *dummy)
{
- int rc;
+ while (!kthread_should_stop()) {
- while (1) {
- rc = wait_event_interruptible(cmm_thread_wait,
- (cmm_pages != cmm_pages_target ||
- cmm_timed_pages != cmm_timed_pages_target ||
- kthread_should_stop()));
- if (kthread_should_stop() || rc == -ERESTARTSYS) {
- cmm_pages_target = cmm_pages;
- cmm_timed_pages_target = cmm_timed_pages;
+ if (kthread_should_stop())
break;
- }
if (cmm_pages_target > cmm_pages) {
if (cmm_alloc_pages(1, &cmm_pages, &cmm_page_list))
cmm_pages_target = cmm_pages;
@@ -183,48 +170,19 @@ cmm_thread(void *dummy)
cmm_free_pages(1, &cmm_timed_pages,
&cmm_timed_page_list);
}
- if (cmm_timed_pages > 0 && !timer_pending(&cmm_timer))
- cmm_set_timer();
+
+ __set_current_state(TASK_INTERRUPTIBLE);
+ schedule_timeout(cmm_timeout_seconds*HZ);
}
+
return 0;
}
static void
cmm_kick_thread(void)
{
- wake_up(&cmm_thread_wait);
-}
-
-static void
-cmm_set_timer(void)
-{
- if (cmm_timed_pages_target <= 0 || cmm_timeout_seconds <= 0) {
- if (timer_pending(&cmm_timer))
- del_timer(&cmm_timer);
- return;
- }
- if (timer_pending(&cmm_timer)) {
- if (mod_timer(&cmm_timer, jiffies + cmm_timeout_seconds*HZ))
- return;
- }
- cmm_timer.function = cmm_timer_fn;
- cmm_timer.data = 0;
- cmm_timer.expires = jiffies + cmm_timeout_seconds*HZ;
- add_timer(&cmm_timer);
-}
-
-static void
-cmm_timer_fn(unsigned long ignored)
-{
- long nr;
-
- nr = cmm_timed_pages_target - cmm_timeout_pages;
- if (nr < 0)
- cmm_timed_pages_target = 0;
- else
- cmm_timed_pages_target = nr;
- cmm_kick_thread();
- cmm_set_timer();
+ if (cmm_thread_ptr)
+ wake_up_process(cmm_thread_ptr);
}
void
@@ -258,7 +216,6 @@ cmm_set_timeout(long nr, long seconds)
{
cmm_timeout_pages = nr;
cmm_timeout_seconds = seconds;
- cmm_set_timer();
}
static inline int
@@ -450,12 +407,12 @@ cmm_init (void)
rc = register_oom_notifier(&cmm_oom_nb);
if (rc < 0)
goto out_oom_notify;
- init_waitqueue_head(&cmm_thread_wait);
- init_timer(&cmm_timer);
cmm_thread_ptr = kthread_run(cmm_thread, NULL, "cmmthread");
- rc = IS_ERR(cmm_thread_ptr) ? PTR_ERR(cmm_thread_ptr) : 0;
- if (!rc)
+ if (IS_ERR(cmm_thread_ptr)) {
+ rc = PTR_ERR(cmm_thread_ptr);
+ cmm_thread_ptr = NULL;
goto out;
+ }
/*
* kthread_create failed. undo all the stuff from above again.
*/
--
1.1.6
> below patch on top of -mm2 is wrong (it compiles, but I just noticed
> 2.6.18-rc4-mm2 doesn't boot without this patch either) but hopefully
2.6.18-rc4-mm2 works fine for me. What configuration and machine setup did
you use?
Quoting Heiko Carstens ([email protected]):
> > below patch on top of -mm2 is wrong (it compiles, but I just noticed
> > 2.6.18-rc4-mm2 doesn't boot without this patch either) but hopefully
>
> 2.6.18-rc4-mm2 works fine for me. What configuration and machine setup did
> you use?
Hmm, with my standard config it actually boots. It fails when I turn
off module support. The guilty config is attached, as well as the
config that boots, and the output when it crashes with the guilty
config.
-serge
On Sun, 2006-08-27 at 13:51 -0500, Serge E. Hallyn wrote:
> Hmm, with my standard config it actually boots. It fails when I turn
> off module support. The guilty config is attached, as well as the
> config that boots, and the output when it crashes with the guilty
> config.
That looks like the crypto initialization problem. Currently the new
crypto driver only works if you compile it as a module.
--
blue skies,
Martin.
Martin Schwidefsky
Linux for zSeries Development & Services
IBM Deutschland Entwicklung GmbH
"Reality continues to ruin my life." - Calvin.
On Fri, 2006-08-25 at 15:03 -0500, Serge E. Hallyn wrote:
> Ok, the patch in -mm does kthread_stop() on module_exit, but still uses
> the timer and cmm_thread_wait.
Yes, the timer and cmm_thread_wait are there to implement the timed page
pool.
> I'm not clear what the timer is actually trying to do, or why there is a
> separate cmm_pages_target and cmm_timed_pages_target. So I'm sure the
> below patch on top of -mm2 is wrong (it compiles, but I just noticed
> 2.6.18-rc4-mm2 doesn't boot without this patch either) but hopefully
> Heiko or Martin can tell me what would be the right way, or implement
> it?
Yes, it is wrong. Trying to "fix" code without understanding it is waste
of time. The while loop in the cmm_thread is supposed to continue until
the target numbers for the standard page pool and the timed page pool
have been reached. Your patch adds a schedule_timeout between every call
to cmm_alloc_pages.
--
blue skies,
Martin.
Martin Schwidefsky
Linux for zSeries Development & Services
IBM Deutschland Entwicklung GmbH
"Reality continues to ruin my life." - Calvin.
> AP instructions not installed.
> BUG: warning at lib/kref.c:32/kref_get()
> 000000000017719a 0000000000000002 0000000000000000 0000000000a03cf0
> 0000000000a03c68 000000000037fc2c 000000000037fc2c 0000000000015dfa
> 0000000000000000 0000000000000000 000000000043ac30 0000000000000000
> 0000000000000000 000000000000000d 0000000000a03c50 0000000000a03cc8
> 0000000000362488 0000000000015dfa 0000000000a03c50 0000000000a03ca0
> Call Trace:
> (Ý<0000000000015d44>¨ show_trace+0x9c/0xb8)
> Ý<0000000000015e18>¨ show_stack+0xb8/0xc8
> Ý<0000000000015e56>¨ dump_stack+0x2e/0x3c
> Ý<0000000000163f70>¨ kref_get+0x50/0x74
> Ý<0000000000162ec6>¨ kobject_get+0x32/0x44
> Ý<0000000000178fd6>¨ get_bus+0x36/0x60
> Ý<0000000000179a12>¨ bus_add_driver+0x3a/0x1f4
> Ý<000000000017ae30>¨ driver_register+0xb0/0xc0
> Ý<000000000021a1fe>¨ ap_driver_register+0x56/0x64
> Ý<00000000004c6ba6>¨ zcrypt_pcicc_init+0x36/0x44
> Ý<000000000001330c>¨ init+0x1bc/0x3a4
> Ý<00000000000184be>¨ kernel_thread_starter+0x6/0xc
> Ý<00000000000184b8>¨ kernel_thread_starter+0x0/0xc
This should be fixed with -mm3. In addition you need this one on top of -mm3:
arch/s390/kernel/time.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Index: linux-2.6.18-rc4-mm3/arch/s390/kernel/time.c
===================================================================
--- linux-2.6.18-rc4-mm3.orig/arch/s390/kernel/time.c 2006-08-28 10:32:45.000000000 +0200
+++ linux-2.6.18-rc4-mm3/arch/s390/kernel/time.c 2006-08-28 10:42:33.000000000 +0200
@@ -85,7 +85,8 @@
{
__u64 now;
- now = (get_clock() - jiffies_timer_cc) >> 12;
+ now = (get_clock() - jiffies_timer_cc) >> 12;
+ now -= (__u64) jiffies * USECS_PER_JIFFY;
return (unsigned long) now;
}
Quoting Heiko Carstens ([email protected]):
> > AP instructions not installed.
> > BUG: warning at lib/kref.c:32/kref_get()
> > 000000000017719a 0000000000000002 0000000000000000 0000000000a03cf0
> > 0000000000a03c68 000000000037fc2c 000000000037fc2c 0000000000015dfa
> > 0000000000000000 0000000000000000 000000000043ac30 0000000000000000
> > 0000000000000000 000000000000000d 0000000000a03c50 0000000000a03cc8
> > 0000000000362488 0000000000015dfa 0000000000a03c50 0000000000a03ca0
> > Call Trace:
> > (Ý<0000000000015d44>¨ show_trace+0x9c/0xb8)
> > Ý<0000000000015e18>¨ show_stack+0xb8/0xc8
> > Ý<0000000000015e56>¨ dump_stack+0x2e/0x3c
> > Ý<0000000000163f70>¨ kref_get+0x50/0x74
> > Ý<0000000000162ec6>¨ kobject_get+0x32/0x44
> > Ý<0000000000178fd6>¨ get_bus+0x36/0x60
> > Ý<0000000000179a12>¨ bus_add_driver+0x3a/0x1f4
> > Ý<000000000017ae30>¨ driver_register+0xb0/0xc0
> > Ý<000000000021a1fe>¨ ap_driver_register+0x56/0x64
> > Ý<00000000004c6ba6>¨ zcrypt_pcicc_init+0x36/0x44
> > Ý<000000000001330c>¨ init+0x1bc/0x3a4
> > Ý<00000000000184be>¨ kernel_thread_starter+0x6/0xc
> > Ý<00000000000184b8>¨ kernel_thread_starter+0x0/0xc
>
> This should be fixed with -mm3. In addition you need this one on top of -mm3:
>
> arch/s390/kernel/time.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> Index: linux-2.6.18-rc4-mm3/arch/s390/kernel/time.c
> ===================================================================
> --- linux-2.6.18-rc4-mm3.orig/arch/s390/kernel/time.c 2006-08-28 10:32:45.000000000 +0200
> +++ linux-2.6.18-rc4-mm3/arch/s390/kernel/time.c 2006-08-28 10:42:33.000000000 +0200
> @@ -85,7 +85,8 @@
> {
> __u64 now;
>
> - now = (get_clock() - jiffies_timer_cc) >> 12;
> + now = (get_clock() - jiffies_timer_cc) >> 12;
> + now -= (__u64) jiffies * USECS_PER_JIFFY;
> return (unsigned long) now;
> }
This patch appears to be actually in -mm3 (at least the git tree I just
fetched), and is does boot fine now.
thanks,
-serge