From: "Luis R. Rodriguez" <mcgrof@kernel.org> To: Bart Van Assche <Bart.VanAssche@wdc.com> Cc: "pavel@ucw.cz" <pavel@ucw.cz>, "darrick.wong@oracle.com" <darrick.wong@oracle.com>, "viro@zeniv.linux.org.uk" <viro@zeniv.linux.org.uk>, "ming.lei@redhat.com" <ming.lei@redhat.com>, "rjw@rjwysocki.net" <rjw@rjwysocki.net>, "mcgrof@kernel.org" <mcgrof@kernel.org>, "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>, "jikos@kernel.org" <jikos@kernel.org>, "len.brown@intel.com" <len.brown@intel.com>, "tytso@mit.edu" <tytso@mit.edu>, "boris.ostrovsky@oracle.com" <boris.ostrovsky@oracle.com>, "ONeukum@suse.com" <ONeukum@suse.com>, "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "nborisov@suse.com" <nborisov@suse.com>, "oleg.b.antonyan@gmail.com" <oleg.b.antonyan@gmail.com>, "linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>, "linux-xfs@vger.kernel.org" <linux-xfs@vger.kernel.org>, "jgross@suse.com" <jgross@suse.com>, "martin.petersen@oracle.com" <martin.petersen@oracle.com>, "oleksandr@natalenko.name" <oleksandr@natalenko.name>, "todd.e.brandt@linux.intel.com" <todd.e.brandt@linux.intel.com>, "jack@suse.cz" <jack@suse.cz> Subject: Re: [RFC 2/5] fs: freeze on suspend and thaw on resume Date: Tue, 3 Oct 2017 22:23:35 +0200 [thread overview] Message-ID: <20171003202335.GF2294@wotan.suse.de> (raw) In-Reply-To: <1507060941.2567.6.camel@wdc.com> On Tue, Oct 03, 2017 at 08:02:22PM +0000, Bart Van Assche wrote: > On Tue, 2017-10-03 at 11:53 -0700, Luis R. Rodriguez wrote: > > +static bool super_allows_freeze(struct super_block *sb) > > +{ > > + return !!(sb->s_type->fs_flags & FS_FREEZE_ON_SUSPEND); > > +} > > A minor comment: if "!!" would be left out the compiler will perform the > conversion from int to bool implicitly For all compilers? > so I propose to leave out the "!!" and parentheses. OK! > Anyway, I agree with the approach of this patch and I think > that freezing filesystems before processes are frozen would be a big step > forward. Great! But please note, the current implementation calls fs_suspend_freeze() *after* try_to_freeze_tasks(), ie: this implementation freezes userspace and only after then filesystems. Order will be *critical* here to get right, so we should definitely figure out if this is definitely the right place (TM) to call fs_suspend_freeze(). Lastly, a final minor implementation note: I think using a PM notifier would have been much cleaner, in fact it was my the way I originally implemented this orthogonally to Jiri's work, however to get this right the semantics of __pm_notifier_call_chain() would need to be expanded with another state, perhaps PM_USERSPACE_FROZEN, for example. I decided in the end a new state was not worth it give we would just have one user: fs freezing. So to be clear using a notifier to wrap this code into the fs code and not touching kernel/power/process.c is not possible with today's semantics nor do I think its worth it to expand on these semantics. This approach is explicit about order and requirements for those that should care: those that will maintain kernel/power/process.c and friends. Having this in a notifier would shift this implicitly. Luis
WARNING: multiple messages have this Message-ID (diff)
From: "Luis R. Rodriguez" <mcgrof@kernel.org> To: Bart Van Assche <Bart.VanAssche@wdc.com> Cc: "pavel@ucw.cz" <pavel@ucw.cz>, "darrick.wong@oracle.com" <darrick.wong@oracle.com>, "viro@zeniv.linux.org.uk" <viro@zeniv.linux.org.uk>, "ming.lei@redhat.com" <ming.lei@redhat.com>, "rjw@rjwysocki.net" <rjw@rjwysocki.net>, "mcgrof@kernel.org" <mcgrof@kernel.org>, "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>, "jikos@kernel.org" <jikos@kernel.org>, "len.brown@intel.com" <len.brown@intel.com>, "tytso@mit.edu" <tytso@mit.edu>, "boris.ostrovsky@oracle.com" <boris.ostrovsky@oracle.com>, "ONeukum@suse.com" <ONeukum@suse.com>, "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "nborisov@suse.com" <nborisov@suse.com>, "oleg.b.antonyan@gmail.com" <oleg.b.antonyan@gmail.com>l Subject: Re: [RFC 2/5] fs: freeze on suspend and thaw on resume Date: Tue, 3 Oct 2017 22:23:35 +0200 [thread overview] Message-ID: <20171003202335.GF2294@wotan.suse.de> (raw) In-Reply-To: <1507060941.2567.6.camel@wdc.com> On Tue, Oct 03, 2017 at 08:02:22PM +0000, Bart Van Assche wrote: > On Tue, 2017-10-03 at 11:53 -0700, Luis R. Rodriguez wrote: > > +static bool super_allows_freeze(struct super_block *sb) > > +{ > > + return !!(sb->s_type->fs_flags & FS_FREEZE_ON_SUSPEND); > > +} > > A minor comment: if "!!" would be left out the compiler will perform the > conversion from int to bool implicitly For all compilers? > so I propose to leave out the "!!" and parentheses. OK! > Anyway, I agree with the approach of this patch and I think > that freezing filesystems before processes are frozen would be a big step > forward. Great! But please note, the current implementation calls fs_suspend_freeze() *after* try_to_freeze_tasks(), ie: this implementation freezes userspace and only after then filesystems. Order will be *critical* here to get right, so we should definitely figure out if this is definitely the right place (TM) to call fs_suspend_freeze(). Lastly, a final minor implementation note: I think using a PM notifier would have been much cleaner, in fact it was my the way I originally implemented this orthogonally to Jiri's work, however to get this right the semantics of __pm_notifier_call_chain() would need to be expanded with another state, perhaps PM_USERSPACE_FROZEN, for example. I decided in the end a new state was not worth it give we would just have one user: fs freezing. So to be clear using a notifier to wrap this code into the fs code and not touching kernel/power/process.c is not possible with today's semantics nor do I think its worth it to expand on these semantics. This approach is explicit about order and requirements for those that should care: those that will maintain kernel/power/process.c and friends. Having this in a notifier would shift this implicitly. Luis
next prev parent reply other threads:[~2017-10-03 20:23 UTC|newest] Thread overview: 74+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-10-03 18:53 [RFC 0/5] fs: replace kthread freezing with filesystem freeze/thaw Luis R. Rodriguez 2017-10-03 18:53 ` [RFC 1/5] fs: add iterate_supers_reverse() Luis R. Rodriguez 2017-10-03 18:53 ` [RFC 2/5] fs: freeze on suspend and thaw on resume Luis R. Rodriguez 2017-10-03 20:02 ` Bart Van Assche 2017-10-03 20:02 ` Bart Van Assche 2017-10-03 20:23 ` Luis R. Rodriguez [this message] 2017-10-03 20:23 ` Luis R. Rodriguez 2017-10-03 20:32 ` Bart Van Assche 2017-10-03 20:32 ` Bart Van Assche 2017-10-03 20:32 ` Bart Van Assche 2017-10-03 20:32 ` Bart Van Assche 2017-10-03 20:39 ` Luis R. Rodriguez 2017-10-03 20:39 ` Luis R. Rodriguez 2017-10-03 20:06 ` Jiri Kosina 2017-10-03 20:58 ` Dave Chinner 2017-10-03 21:16 ` Luis R. Rodriguez 2017-10-03 18:53 ` [RFC 3/5] xfs: allow fs freeze on suspend/hibernation Luis R. Rodriguez 2017-10-03 18:53 ` [RFC 4/5] ext4: add fs freezing support " Luis R. Rodriguez 2017-10-03 19:59 ` Theodore Ts'o 2017-10-03 20:13 ` Luis R. Rodriguez 2017-10-04 1:42 ` Theodore Ts'o 2017-10-04 7:05 ` Dave Chinner 2017-10-04 15:25 ` Bart Van Assche 2017-10-04 15:25 ` Bart Van Assche 2017-10-04 15:25 ` Bart Van Assche 2017-10-04 15:25 ` Bart Van Assche 2017-10-04 16:48 ` Theodore Ts'o 2017-10-04 22:22 ` Dave Chinner 2017-10-03 18:53 ` [RFC 5/5] pm: remove kernel thread freezing Luis R. Rodriguez 2017-10-03 18:59 ` Rafael J. Wysocki 2017-10-03 21:15 ` Rafael J. Wysocki 2017-10-04 0:47 ` Luis R. Rodriguez 2017-10-04 1:03 ` Bart Van Assche 2017-10-04 1:03 ` Bart Van Assche 2017-10-04 1:03 ` Bart Van Assche 2017-11-29 23:05 ` Luis R. Rodriguez 2017-11-29 23:05 ` Luis R. Rodriguez 2017-10-04 7:18 ` Dave Chinner 2017-10-03 20:12 ` Pavel Machek 2017-10-03 20:15 ` Jiri Kosina 2017-10-03 20:21 ` Pavel Machek 2017-10-03 20:38 ` Jiri Kosina 2017-10-03 20:41 ` Rafael J. Wysocki 2017-10-03 20:57 ` Pavel Machek 2017-10-03 21:00 ` Jiri Kosina 2017-10-03 21:09 ` Shuah Khan 2017-10-03 21:18 ` Luis R. Rodriguez 2017-10-03 20:49 ` Luis R. Rodriguez 2017-10-06 12:07 ` Pavel Machek 2017-10-06 12:54 ` Theodore Ts'o 2017-10-03 20:13 ` Bart Van Assche 2017-10-03 20:13 ` Bart Van Assche 2017-10-03 20:17 ` Jiri Kosina 2017-10-03 20:17 ` Jiri Kosina 2017-10-03 20:21 ` Bart Van Assche 2017-10-03 20:21 ` Bart Van Assche 2017-10-03 20:21 ` Bart Van Assche 2017-10-03 20:24 ` Jiri Kosina 2017-10-03 20:24 ` Jiri Kosina 2017-10-03 20:27 ` Luis R. Rodriguez 2017-10-03 20:27 ` Luis R. Rodriguez 2017-10-03 20:51 ` Jiri Kosina 2017-10-03 20:51 ` Jiri Kosina 2017-10-03 21:04 ` Dave Chinner 2017-10-03 21:07 ` Luis R. Rodriguez 2017-10-04 6:07 ` Hannes Reinecke 2017-10-03 19:33 ` [RFC 0/5] fs: replace kthread freezing with filesystem freeze/thaw Ming Lei 2017-10-03 20:05 ` Luis R. Rodriguez 2017-10-03 20:47 ` Matthew Wilcox 2017-10-03 20:54 ` Luis R. Rodriguez 2017-10-03 20:59 ` Bart Van Assche 2017-10-03 20:59 ` Bart Van Assche 2017-10-03 20:59 ` Bart Van Assche 2017-10-04 15:43 ` Ming Lei
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=20171003202335.GF2294@wotan.suse.de \ --to=mcgrof@kernel.org \ --cc=Bart.VanAssche@wdc.com \ --cc=ONeukum@suse.com \ --cc=boris.ostrovsky@oracle.com \ --cc=darrick.wong@oracle.com \ --cc=jack@suse.cz \ --cc=jgross@suse.com \ --cc=jikos@kernel.org \ --cc=len.brown@intel.com \ --cc=linux-block@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=linux-xfs@vger.kernel.org \ --cc=martin.petersen@oracle.com \ --cc=ming.lei@redhat.com \ --cc=nborisov@suse.com \ --cc=oleg.b.antonyan@gmail.com \ --cc=oleksandr@natalenko.name \ --cc=pavel@ucw.cz \ --cc=rjw@rjwysocki.net \ --cc=todd.e.brandt@linux.intel.com \ --cc=tytso@mit.edu \ --cc=viro@zeniv.linux.org.uk \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.