* [Patch] JBD and JBD2 missing set_freezable()
@ 2012-02-03 8:59 Nigel Cunningham
2012-03-04 22:51 ` [linux-pm] " Rafael J. Wysocki
0 siblings, 1 reply; 4+ messages in thread
From: Nigel Cunningham @ 2012-02-03 8:59 UTC (permalink / raw
To: Linux PM mailing list
Hi all.
With the latest and greatest changes to the freezer, I started seeing
panics that were caused by jbd2 running post-process freezing and
hitting the canary BUG_ON for non-TuxOnIce I/O submission. I've traced
this back to a lack of set_freezable calls in both jbd and jbd2. Since
they're clearly meant to be frozen (there are tests for freezing()), I
submit the following patch to add the missing calls.
Signed-off-by: Nigel Cunningham <nigel@tuxonice.net>
diff --git a/fs/jbd/journal.c b/fs/jbd/journal.c
index 59c09f9..89cd985 100644
--- a/fs/jbd/journal.c
+++ b/fs/jbd/journal.c
@@ -129,6 +129,8 @@ static int kjournald(void *arg)
setup_timer(&journal->j_commit_timer, commit_timeout,
(unsigned long)current);
+ set_freezable();
+
/* Record that the journal thread is running */
journal->j_task = current;
wake_up(&journal->j_wait_done_commit);
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index c0a5f9f..663e47c 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -139,6 +139,8 @@ static int kjournald2(void *arg)
setup_timer(&journal->j_commit_timer, commit_timeout,
(unsigned long)current);
+ set_freezable();
+
/* Record that the journal thread is running */
journal->j_task = current;
wake_up(&journal->j_wait_done_commit);
--
Evolution (n): A hypothetical process whereby improbable
events occur with alarming frequency, order arises from chaos, and
no one is given credit.
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [linux-pm] [Patch] JBD and JBD2 missing set_freezable()
2012-02-03 8:59 [Patch] JBD and JBD2 missing set_freezable() Nigel Cunningham
@ 2012-03-04 22:51 ` Rafael J. Wysocki
2012-03-05 10:28 ` Jan Kara
0 siblings, 1 reply; 4+ messages in thread
From: Rafael J. Wysocki @ 2012-03-04 22:51 UTC (permalink / raw
To: linux-pm; +Cc: Nigel Cunningham, Jan Kara, linux-fsdevel, LKML, linux-ext4
On Friday, February 03, 2012, Nigel Cunningham wrote:
> Hi all.
>
> With the latest and greatest changes to the freezer, I started seeing
> panics that were caused by jbd2 running post-process freezing and
> hitting the canary BUG_ON for non-TuxOnIce I/O submission. I've traced
> this back to a lack of set_freezable calls in both jbd and jbd2. Since
> they're clearly meant to be frozen (there are tests for freezing()), I
> submit the following patch to add the missing calls.
>
> Signed-off-by: Nigel Cunningham <nigel@tuxonice.net>
Well, I wonder what the filesystems people think about that.
Thanks,
Rafael
> diff --git a/fs/jbd/journal.c b/fs/jbd/journal.c
> index 59c09f9..89cd985 100644
> --- a/fs/jbd/journal.c
> +++ b/fs/jbd/journal.c
> @@ -129,6 +129,8 @@ static int kjournald(void *arg)
> setup_timer(&journal->j_commit_timer, commit_timeout,
> (unsigned long)current);
>
> + set_freezable();
> +
> /* Record that the journal thread is running */
> journal->j_task = current;
> wake_up(&journal->j_wait_done_commit);
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index c0a5f9f..663e47c 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -139,6 +139,8 @@ static int kjournald2(void *arg)
> setup_timer(&journal->j_commit_timer, commit_timeout,
> (unsigned long)current);
>
> + set_freezable();
> +
> /* Record that the journal thread is running */
> journal->j_task = current;
> wake_up(&journal->j_wait_done_commit);
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [linux-pm] [Patch] JBD and JBD2 missing set_freezable()
2012-03-04 22:51 ` [linux-pm] " Rafael J. Wysocki
@ 2012-03-05 10:28 ` Jan Kara
2012-03-10 21:42 ` Rafael J. Wysocki
0 siblings, 1 reply; 4+ messages in thread
From: Jan Kara @ 2012-03-05 10:28 UTC (permalink / raw
To: Rafael J. Wysocki
Cc: linux-pm, Nigel Cunningham, Jan Kara, linux-fsdevel, LKML,
linux-ext4
On Sun 04-03-12 23:51:07, Rafael J. Wysocki wrote:
> On Friday, February 03, 2012, Nigel Cunningham wrote:
> > Hi all.
> >
> > With the latest and greatest changes to the freezer, I started seeing
> > panics that were caused by jbd2 running post-process freezing and
> > hitting the canary BUG_ON for non-TuxOnIce I/O submission. I've traced
> > this back to a lack of set_freezable calls in both jbd and jbd2. Since
> > they're clearly meant to be frozen (there are tests for freezing()), I
> > submit the following patch to add the missing calls.
> >
> > Signed-off-by: Nigel Cunningham <nigel@tuxonice.net>
>
> Well, I wonder what the filesystems people think about that.
Nigel is right that threads are meant to be freezable. If below patch
is needed for that, you have my ack.
Just two notes:
1) Freezing the journal thread will effectively block filesystem activity
(not immediately but once transaction fills up) so you should better make
sure you don't need to do anything with the filesystem after freezing the
thread.
2) Is freezing journal thread still needed when we freeze filesystem in
suspend code? Because no IO should happen once filesystem is frozen...
Honza
>
> Thanks,
> Rafael
>
>
> > diff --git a/fs/jbd/journal.c b/fs/jbd/journal.c
> > index 59c09f9..89cd985 100644
> > --- a/fs/jbd/journal.c
> > +++ b/fs/jbd/journal.c
> > @@ -129,6 +129,8 @@ static int kjournald(void *arg)
> > setup_timer(&journal->j_commit_timer, commit_timeout,
> > (unsigned long)current);
> >
> > + set_freezable();
> > +
> > /* Record that the journal thread is running */
> > journal->j_task = current;
> > wake_up(&journal->j_wait_done_commit);
> > diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> > index c0a5f9f..663e47c 100644
> > --- a/fs/jbd2/journal.c
> > +++ b/fs/jbd2/journal.c
> > @@ -139,6 +139,8 @@ static int kjournald2(void *arg)
> > setup_timer(&journal->j_commit_timer, commit_timeout,
> > (unsigned long)current);
> >
> > + set_freezable();
> > +
> > /* Record that the journal thread is running */
> > journal->j_task = current;
> > wake_up(&journal->j_wait_done_commit);
> >
>
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [linux-pm] [Patch] JBD and JBD2 missing set_freezable()
2012-03-05 10:28 ` Jan Kara
@ 2012-03-10 21:42 ` Rafael J. Wysocki
0 siblings, 0 replies; 4+ messages in thread
From: Rafael J. Wysocki @ 2012-03-10 21:42 UTC (permalink / raw
To: Jan Kara; +Cc: linux-pm, Nigel Cunningham, linux-fsdevel, LKML, linux-ext4
Hi,
Sorry for the delay.
On Monday, March 05, 2012, Jan Kara wrote:
> On Sun 04-03-12 23:51:07, Rafael J. Wysocki wrote:
> > On Friday, February 03, 2012, Nigel Cunningham wrote:
> > > Hi all.
> > >
> > > With the latest and greatest changes to the freezer, I started seeing
> > > panics that were caused by jbd2 running post-process freezing and
> > > hitting the canary BUG_ON for non-TuxOnIce I/O submission. I've traced
> > > this back to a lack of set_freezable calls in both jbd and jbd2. Since
> > > they're clearly meant to be frozen (there are tests for freezing()), I
> > > submit the following patch to add the missing calls.
> > >
> > > Signed-off-by: Nigel Cunningham <nigel@tuxonice.net>
> >
> > Well, I wonder what the filesystems people think about that.
> Nigel is right that threads are meant to be freezable. If below patch
> is needed for that, you have my ack.
Yes, it is, thanks! I'm going to push it for v3.4 along with PM updates.
> Just two notes:
> 1) Freezing the journal thread will effectively block filesystem activity
> (not immediately but once transaction fills up) so you should better make
> sure you don't need to do anything with the filesystem after freezing the
> thread.
Sure, no worries. :-)
> 2) Is freezing journal thread still needed when we freeze filesystem in
> suspend code? Because no IO should happen once filesystem is frozen...
It won't be strictly necessary, but it won't hurt either. However, until
we start freezing filesystems in suspend code, it is needed.
Thanks,
Rafael
> > > diff --git a/fs/jbd/journal.c b/fs/jbd/journal.c
> > > index 59c09f9..89cd985 100644
> > > --- a/fs/jbd/journal.c
> > > +++ b/fs/jbd/journal.c
> > > @@ -129,6 +129,8 @@ static int kjournald(void *arg)
> > > setup_timer(&journal->j_commit_timer, commit_timeout,
> > > (unsigned long)current);
> > >
> > > + set_freezable();
> > > +
> > > /* Record that the journal thread is running */
> > > journal->j_task = current;
> > > wake_up(&journal->j_wait_done_commit);
> > > diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> > > index c0a5f9f..663e47c 100644
> > > --- a/fs/jbd2/journal.c
> > > +++ b/fs/jbd2/journal.c
> > > @@ -139,6 +139,8 @@ static int kjournald2(void *arg)
> > > setup_timer(&journal->j_commit_timer, commit_timeout,
> > > (unsigned long)current);
> > >
> > > + set_freezable();
> > > +
> > > /* Record that the journal thread is running */
> > > journal->j_task = current;
> > > wake_up(&journal->j_wait_done_commit);
> > >
> >
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-03-10 21:37 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-03 8:59 [Patch] JBD and JBD2 missing set_freezable() Nigel Cunningham
2012-03-04 22:51 ` [linux-pm] " Rafael J. Wysocki
2012-03-05 10:28 ` Jan Kara
2012-03-10 21:42 ` Rafael J. Wysocki
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.