| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 25 Aug 2006 15:03:59 -0500
From: "Serge E. Hallyn" <serue@...ibm.com>
To: Christoph Hellwig <hch@...radead.org>,
"Serge E. Hallyn" <serue@...ibm.com>,
lkml <linux-kernel@...r.kernel.org>, schwidefsky@...ibm.com
Subject: Re: [PATCH 1/3] kthread: update s390 cmm driver to use kthread
Quoting Christoph Hellwig (hch@...radead.org):
> 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 <serue@...ibm.com>
---
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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists