From: Jiri Kosina <jkosina@suse.cz>
To: Chris Mason <clm@fb.com>, Josef Bacik <jbacik@fb.com>,
David Sterba <dsterba@suse.com>
Cc: linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org,
jack@suse.cz, mhocko@suse.cz
Subject: Re: [PATCH 1/2] btrfs: cleaner_kthread() doesn't need explicit freeze
Date: Tue, 15 Mar 2016 14:07:23 +0100 (CET) [thread overview]
Message-ID: <alpine.LNX.2.00.1603151400460.3656@cbobk.fhfr.pm> (raw)
In-Reply-To: <alpine.LNX.2.00.1603151126080.3656@cbobk.fhfr.pm>
On Tue, 15 Mar 2016, Jiri Kosina wrote:
> cleaner_kthread() is not marked freezable, and therefore calling
> try_to_freeze() in its context is a pointless no-op.
>
> In addition to that, as has been clearly demonstrated by 80ad623edd2d
> ("Revert "btrfs: clear PF_NOFREEZE in cleaner_kthread()"), it's perfectly
> valid / legal for cleaner_kthread() to stay scheduled out in an arbitrary
> place during suspend (in that particular example that was waiting for
> reading of extent pages), so there is no need to leave any traces of
> freezer in this kthread.
Given some questions I've received offline, let me clarify a little bit
more here.
Currently, the try_to_freeze() call is completely useless here, because it
will never actually try to freeze the kthread (as it's PF_NOFREEZE).
When attempted to make the kthread properly freezable, it turned out (see
e.g. 80ad623edd2d) that it's actually sleeping in various places during
suspend for long periods of time (my guess would be that it doesn't really
matter whether the cleaning happens before or after suspend, but this'd be
something I'd like to have clarified from btrfs folks).
So in a nutshell, this patch (a) doesn't make things worse, as it's an
equivalent code transformation (b) brings more sanity to how the kthread
freezing API is used throughout the kernel.
It might very well be that the code was broken before; but it's not more
broken after this patch, and the API usage is sane.
The ultimate goal is first to bring some sanity into how the freezer API
is used throughout the kernel, and then eventually get rid of it
completely in favor of fs freezing (currently it's not even possible to
analyze all the uses in the kernel, as there are way too many and most of
them are totally broken).
--
Jiri Kosina
SUSE Labs
prev parent reply other threads:[~2016-03-15 13:07 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-15 10:28 [PATCH 1/2] btrfs: cleaner_kthread() doesn't need explicit freeze Jiri Kosina
2016-03-15 10:28 ` [PATCH 2/2] btrfs: transaction_kthread() is not freezable Jiri Kosina
2016-03-15 13:07 ` Jiri Kosina [this message]
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=alpine.LNX.2.00.1603151400460.3656@cbobk.fhfr.pm \
--to=jkosina@suse.cz \
--cc=clm@fb.com \
--cc=dsterba@suse.com \
--cc=jack@suse.cz \
--cc=jbacik@fb.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mhocko@suse.cz \
/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
Be 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.