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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:	Sat, 10 Mar 2012 22:42:00 +0100
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	Jan Kara <jack@...e.cz>
Cc:	linux-pm@...ts.linux-foundation.org,
	Nigel Cunningham <nigel@...onice.net>,
	linux-fsdevel@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
	linux-ext4@...r.kernel.org
Subject: Re: [linux-pm] [Patch] JBD and JBD2 missing set_freezable()

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@...onice.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);
> > > 
> > 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists