LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Nigel Cunningham <ncunningham@crca.org.au>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: rjw@sisk.pl, linux-pm@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org
Subject: Re: [linux-pm] Freezer: Don't count threads waiting for frozen filesystems.
Date: Tue, 28 Oct 2008 09:13:27 +1100	[thread overview]
Message-ID: <1225145607.26724.54.camel@nigel-laptop> (raw)
In-Reply-To: <E1KuZKt-0003jl-2N@pomaz-ex.szeredi.hu>

Hi.

On Mon, 2008-10-27 at 22:09 +0100, Miklos Szeredi wrote:
> On Tue, 28 Oct 2008, Nigel Cunningham wrote:
> > Hi.
> > 
> > On Mon, 2008-10-27 at 13:38 +0100, Miklos Szeredi wrote:
> > > On Mon, 27 Oct 2008, Nigel Cunningham wrote:
> > > > Hi.
> > > > 
> > > > On Mon, 2008-10-27 at 12:37 +0100, Rafael J. Wysocki wrote:
> > > > > Well, I guess it's better if you post the entire thing so that we can see
> > > > > what the role of the $subject patch is in it, even if this patch finally gets
> > > > > merged separately.
> > > > 
> > > > Ah.. that makes me see how vfs_check_frozen was getting triggered...
> > > > (fs/namei.c, below).
> > > 
> > > Nigel, thanks for the patch, it makes thinks a lot clearer.
> > > 
> > > >From a quick look through the patch it seems to solve a bunch of cases
> > > where new fuse requests during the freezing could cause problems.  But
> > > it doesn't do anything with requests that are already under way when
> > > the freezing starts, which would still result in all the same
> > > problems.
> > > 
> > > Take this scenario:
> > > 
> > >  1) process A does rename("/mnt/fuse/a", "/mnt/fuse/b")
> > >  2) request goes to process B serving the fuse filesystem
> > >  3) filesystems are frozen, no new fuse requests
> > >  4) processes are frozen, let's say B first, then A
> > >  5) freezing A will fail, since it's still waiting for the request to finish
> > 
> > I'll have a look at the code again. I deliberately didn't stop existing
> > requests, but perhaps that's the wrong behaviour.
> > 
> > In the above scenario, process B won't see process A's new request until
> > post-resume if the filesystem is already frozen, so steps 4 & 5 don't
> > happen.
> 
> No, I mean the filesystem is only frozen at 3 not before, so B is
> already processing the request by the time the filesystem gets frozen.
> 
> > Process B will also always be frozen before process A if process
> > A is userspace (most likely in the above scenario) or was mounted after
> > process B. (I've had this patch distributed as is for almost a year,
> > with no problems at all, so I believe I'm right here).
> 
> Both A and B are userspace processes.  The order of freezing userspace
> processes is basically random, there's no way to make sure that
> processes doing work on behalf of a fuse filesystem (B) are frozen
> after processes accessing the fuse filesystem (A).

Sorry. You're right - I was confusing things in what I said, but I do
have a (unconfused) solution:

The answer is to freeze the fuse filesystems first, stopping new
requests (freezing the processes making them) before they are passed on
to userspace and allowing existing requests to complete or freeze. Once
that is done, the userspace fuse processes will be idle, at least as far
as fuse activity is concerned. After fuse activity is stopped, userspace
can be frozen without worrying about what processes are fuse and what
aren't. This is what my patch implements so far.

To deal with requests that are already in progress, I'd suggest three
possibilities, in the order I think they should be preferred.

1) Use wait_event_freezeable[_timeout] in fuse code. Probably preferable
to #2, but I thought of it later :)

2) Use freezer_do_not_count as part of FUSE_MIGHT_FREEZE (resetting
before exiting the callers, of course). If the request doesn't complete
during the freezing period, it must be because the userspace thread was
already frozen. If the request does complete, we're counted again during
the normal freezing of userspace that follows the freezing of
filesystems.

3) Adding a means to check whether processes being frozen are fuse
requests. The code could then wait for fc->num_waiting - (say)
fc->num_frozen == 0.

Regards,

Nigel


  reply	other threads:[~2008-10-27 22:13 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1224886068.6478.21.camel@nigel-laptop>
2008-10-26 20:01 ` Rafael J. Wysocki
2008-10-27 11:12   ` Miklos Szeredi
2008-10-27 11:20     ` Nigel Cunningham
2008-10-27 11:37       ` Rafael J. Wysocki
2008-10-27 11:40         ` Nigel Cunningham
2008-10-27 12:38           ` Miklos Szeredi
2008-10-27 20:59             ` Nigel Cunningham
2008-10-27 21:09               ` Miklos Szeredi
2008-10-27 22:13                 ` Nigel Cunningham [this message]
2008-10-28 20:25                   ` Miklos Szeredi
2008-10-28 21:29                     ` Nigel Cunningham
2008-10-28 21:51                       ` Miklos Szeredi
2008-10-28 22:00                         ` Rafael J. Wysocki
2008-10-28 22:02                           ` Miklos Szeredi
2008-10-28 22:21                             ` Rafael J. Wysocki
2008-10-28 23:21                               ` Miklos Szeredi
2008-10-28 23:59                                 ` Rafael J. Wysocki
2008-10-29  8:10                                   ` Miklos Szeredi
2008-10-29 13:51                                     ` Alan Stern
2008-10-29 14:50                                       ` Miklos Szeredi
2008-10-29 15:28                                         ` Alan Stern
2008-10-29 15:50                                           ` Miklos Szeredi
2008-10-29 16:17                                             ` Alan Stern
2008-10-29 16:10                                           ` Miklos Szeredi
2008-10-29 20:37                                             ` Alan Stern
2008-10-29 21:11                                               ` Rafael J. Wysocki
2008-10-29 21:45                                                 ` Nigel Cunningham
2008-10-29 22:07                                                   ` Rafael J. Wysocki
2008-10-29 23:53                                                     ` Miklos Szeredi
2008-11-09 13:44                                                       ` Pavel Machek
2008-10-29 23:48                                                   ` Miklos Szeredi
2008-10-30 13:04                                                     ` Nigel Cunningham
2008-10-30 13:56                                                       ` Alan Stern
2008-10-30 21:44                                                         ` Nigel Cunningham
2008-10-31  8:49                                                           ` Miklos Szeredi
2008-10-31  9:10                                                             ` Nigel Cunningham
2008-10-31  9:16                                                               ` Miklos Szeredi
2008-10-31 11:28                                                                 ` Nigel Cunningham
2008-10-31 12:44                                                                   ` Miklos Szeredi
2008-10-31 21:11                                                                     ` Nigel Cunningham
2008-10-29 23:43                                               ` Miklos Szeredi
2008-10-30 13:54                                                 ` Alan Stern
2008-10-30 14:39                                                   ` Miklos Szeredi
2008-10-30 17:07                                                     ` Alan Stern
2008-10-30 17:43                                                       ` Miklos Szeredi
2008-10-30 20:17                                                     ` Rafael J. Wysocki
2008-11-15 16:58                                                     ` Pavel Machek
2008-10-29 23:56                                               ` Matthew Garrett
2008-10-28 22:03                         ` Nigel Cunningham
2008-10-28 23:04                     ` Nigel Cunningham
2008-10-28 23:12                       ` Miklos Szeredi
2008-10-28 23:17                         ` Nigel Cunningham
2008-10-28 23:24                           ` Miklos Szeredi
2008-10-28 23:41                             ` Nigel Cunningham
2008-10-28 23:45                               ` Miklos Szeredi
2008-10-28 23:50                                 ` Miklos Szeredi
2008-10-28 23:58                                   ` Nigel Cunningham
2008-10-28 23:54                                 ` Nigel Cunningham
2008-10-27 11:37       ` Miklos Szeredi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1225145607.26724.54.camel@nigel-laptop \
    --to=ncunningham@crca.org.au \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@lists.linux-foundation.org \
    --cc=miklos@szeredi.hu \
    --cc=rjw@sisk.pl \
    --subject='Re: [linux-pm] Freezer: Don'\''t count threads waiting for frozen filesystems.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).