All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Luis Chamberlain <mcgrof@kernel.org>
Cc: rafael@kernel.org, gregkh@linuxfoundation.org,
	viro@zeniv.linux.org.uk, jack@suse.cz, bvanassche@acm.org,
	jeyu@kernel.org, ebiederm@xmission.com, mchehab@kernel.org,
	keescook@chromium.org, linux-fsdevel@vger.kernel.org,
	kernel@tuxforce.de, kexec@lists.infradead.org,
	linux-kernel@vger.kernel.org, Dave Chinner <david@fromorbit.com>
Subject: Re: [RFC v2 1/6] fs: provide unlocked helper for freeze_super()
Date: Tue, 20 Apr 2021 13:03:35 +0100	[thread overview]
Message-ID: <20210420120335.GA3604224@infradead.org> (raw)
In-Reply-To: <20210417001026.23858-2-mcgrof@kernel.org>

On Sat, Apr 17, 2021 at 12:10:21AM +0000, Luis Chamberlain wrote:
> freeze_super() holds a write lock, however we wish to also enable
> callers which already hold the write lock. To do this provide a helper
> and make freeze_super() use it. This way, all that freeze_super() does
> now is lock handling and active count management.

Can we take a step back and think about this a bit more?

freeze_super() has three callers:

 1) freeze_bdev
 2) ioctl_fsfreeze
 3) freeze_store (in gfs2)

The first gets its reference from get_active_super, and is the only
caller of get_active_super.  So IMHO we should just not drop the lock
in get_active_super and directly call the unlocked version.

The other two really should just call grab_super to get an active
reference and s_umount.

In other words: I don't think we need both variants, just move the
locking and s_active acquisition out of free_super.  Same for the
thaw side.

WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@infradead.org>
To: Luis Chamberlain <mcgrof@kernel.org>
Cc: rafael@kernel.org, gregkh@linuxfoundation.org,
	viro@zeniv.linux.org.uk, jack@suse.cz, bvanassche@acm.org,
	jeyu@kernel.org, ebiederm@xmission.com, mchehab@kernel.org,
	keescook@chromium.org, linux-fsdevel@vger.kernel.org,
	kernel@tuxforce.de, kexec@lists.infradead.org,
	linux-kernel@vger.kernel.org, Dave Chinner <david@fromorbit.com>
Subject: Re: [RFC v2 1/6] fs: provide unlocked helper for freeze_super()
Date: Tue, 20 Apr 2021 13:03:35 +0100	[thread overview]
Message-ID: <20210420120335.GA3604224@infradead.org> (raw)
In-Reply-To: <20210417001026.23858-2-mcgrof@kernel.org>

On Sat, Apr 17, 2021 at 12:10:21AM +0000, Luis Chamberlain wrote:
> freeze_super() holds a write lock, however we wish to also enable
> callers which already hold the write lock. To do this provide a helper
> and make freeze_super() use it. This way, all that freeze_super() does
> now is lock handling and active count management.

Can we take a step back and think about this a bit more?

freeze_super() has three callers:

 1) freeze_bdev
 2) ioctl_fsfreeze
 3) freeze_store (in gfs2)

The first gets its reference from get_active_super, and is the only
caller of get_active_super.  So IMHO we should just not drop the lock
in get_active_super and directly call the unlocked version.

The other two really should just call grab_super to get an active
reference and s_umount.

In other words: I don't think we need both variants, just move the
locking and s_active acquisition out of free_super.  Same for the
thaw side.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

  reply	other threads:[~2021-04-20 12:06 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-17  0:10 [RFC v2 0/6] vfs: provide automatic kernel freeze / resume Luis Chamberlain
2021-04-17  0:10 ` Luis Chamberlain
2021-04-17  0:10 ` [RFC v2 1/6] fs: provide unlocked helper for freeze_super() Luis Chamberlain
2021-04-17  0:10   ` Luis Chamberlain
2021-04-20 12:03   ` Christoph Hellwig [this message]
2021-04-20 12:03     ` Christoph Hellwig
2021-04-17  0:10 ` [RFC v2 2/6] fs: add frozen sb state helpers Luis Chamberlain
2021-04-17  0:10   ` Luis Chamberlain
2021-04-17  0:10 ` [RFC v2 3/6] fs: add a helper for thaw_super_locked() which does not unlock Luis Chamberlain
2021-04-17  0:10   ` Luis Chamberlain
2021-04-17  0:10 ` [RFC v2 4/6] fs: distinguish between user initiated freeze and kernel initiated freeze Luis Chamberlain
2021-04-17  0:10   ` Luis Chamberlain
2021-04-20 12:46   ` Christoph Hellwig
2021-04-20 12:46     ` Christoph Hellwig
2021-04-17  0:10 ` [RFC v2 5/6] fs: add iterate_supers_excl() and iterate_supers_reverse_excl() Luis Chamberlain
2021-04-17  0:10   ` Luis Chamberlain
2021-04-17  0:10 ` [RFC v2 6/6] fs: add automatic kernel fs freeze / thaw and remove kthread freezing Luis Chamberlain
2021-04-17  0:10   ` Luis Chamberlain
2021-04-20 12:59   ` Christoph Hellwig
2021-04-20 12:59     ` Christoph Hellwig
2021-04-20 18:47     ` Luis Chamberlain
2021-04-20 18:47       ` Luis Chamberlain
2023-01-10  2:11       ` Luis Chamberlain
2023-01-10  2:11         ` Luis Chamberlain

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=20210420120335.GA3604224@infradead.org \
    --to=hch@infradead.org \
    --cc=bvanassche@acm.org \
    --cc=david@fromorbit.com \
    --cc=ebiederm@xmission.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jack@suse.cz \
    --cc=jeyu@kernel.org \
    --cc=keescook@chromium.org \
    --cc=kernel@tuxforce.de \
    --cc=kexec@lists.infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=mchehab@kernel.org \
    --cc=rafael@kernel.org \
    --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: 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.