Re: [PATCH] jffs2_gcd_mtd3, Stopping kernel threads timed out

Uli Luckas
2007-06-26T15:34:34Z

On Tuesday, 26. June 2007, Josh Boyer wrote:
> On Tue, 2007-06-26 at 15:06 +0200, Uli Luckas wrote:
> > On Tuesday, 19. June 2007, Uli Luckas wrote:
> > > Hi,
> > > is there anything in the way of apllying this patch?
> >
> > Please, can someone tell me what it takes to get a jffs2 bug fix into the
> > kernel? Or even reviewed? This patch should even get into 2.6.22. Am I
> > doing something wrong here?
>
> You aren't doing anything wrong.  David is away on business.
>
does that mean, we won't get any fixes applied, before David is back? And does
anyone know, when he will be back?

> The one thing I would hesitate with about your patch is that you've now
> broken the existing behavior of the thread in regards to SIGHUP.  You
> used to be able to send the threads a SIGHUP and it would do a single
> pass.  With your patch, it won't do that unless the thresholds are
> right.  Being able to send SIGHUP to force a GC pass is a pretty useful
> thing for debugging, etc.
>
thanks Josh, alot for reviewing the patch. Following your advise, I force GC
now, if SIGHUP was received.
The patch still introduces a change in sematics though for all other signals
(except SIGKILL). But I feel, the new behavior is more what one would expect.
In case, the old behavior is desired, it is simple enough to set force_gc in
the coresponding branches.

regards,
Uli

Signed-off-by: Uli Luckas <u.luckas@road.de>

diff --git a/fs/jffs2/background.c b/fs/jffs2/background.c
index 888f236..1843786 100644
--- a/fs/jffs2/background.c
+++ b/fs/jffs2/background.c
@@ -71,6 +71,7 @@ void jffs2_stop_garbage_collect_thread(struct jffs2_sb_info *c)

static int jffs2_garbage_collect_thread(void *_c)
{
+ int force_gc;
struct jffs2_sb_info *c = _c;

daemonize("jffs2_gcd_mtd%d", c->mtd->index);
@@ -96,9 +97,6 @@ static int jffs2_garbage_collect_thread(void *_c)
schedule();
}

- if (try_to_freeze())
- continue;
-
/* This thread is purely an optimisation. But if it runs when
other things could be running, it actually makes things a
lot worse. Use yield() and put it at the back of the runqueue
@@ -109,10 +107,14 @@ static int jffs2_garbage_collect_thread(void *_c)

/* Put_super will send a SIGKILL and then wait on the sem.
*/
+ force_gc = 0;
while (signal_pending(current)) {
siginfo_t info;
unsigned long signr;

+ if (try_to_freeze())
+ continue;
+
signr = dequeue_signal_lock(current, &current->blocked, &info);

switch(signr) {
@@ -128,6 +130,7 @@ static int jffs2_garbage_collect_thread(void *_c)

case SIGHUP:
D1(printk(KERN_DEBUG "jffs2_garbage_collect_thread(): SIGHUP received.\n"));
+ force_gc = 1;
break;
default:
D1(printk(KERN_DEBUG "jffs2_garbage_collect_thread(): signal %ld received\n", signr));
@@ -137,7 +140,8 @@ static int jffs2_garbage_collect_thread(void *_c)
disallow_signal(SIGHUP);

D1(printk(KERN_DEBUG "jffs2_garbage_collect_thread(): pass\n"));
- if (jffs2_garbage_collect_pass(c) == -ENOSPC) {
+ if ((force_gc || jffs2_thread_should_wake(c))
+ && jffs2_garbage_collect_pass(c) == -ENOSPC) {
printk(KERN_NOTICE "No space for garbage collection. Aborting GC thread\n");
goto die;
}