All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bob Peterson <rpeterso@redhat.com>
To: Satya Tangirala <satyat@google.com>
Cc: Christoph Hellwig <hch@lst.de>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	Jens Axboe <axboe@kernel.dk>
Subject: Re: [PATCH] fs: Fix freeze_bdev()/thaw_bdev() accounting of bd_fsfreeze_sb
Date: Fri, 8 Jan 2021 09:58:46 -0500 (EST)	[thread overview]
Message-ID: <1568673558.43563383.1610117926358.JavaMail.zimbra@redhat.com> (raw)
In-Reply-To: <879072186.43549344.1610111831181.JavaMail.zimbra@redhat.com>

Hi,

> This is the bigger issue, and I'm not very familiar with this code either,
> so I'll defer to the experts. Yes, it's a change in behavior, but I think
> it makes sense to decrement the bd_fsfreeze_count in this case. Here's why:
> 
> If the blockdev is frozen by freeze_bdev while it's being unmounted, the
> bd_fsfreeze_count is incremented, but the freeze is ignored. Subsequent
> attempts to thaw the device will be ignored but return 0 because the sb
> is not found. When the device is mounted again, calls to freeze_bdev
> will bypass the call to freeze_super for the newly mounted sb, because
> bdev->bd_fsfreeze_count was then incremented from 1 to 2 in freeze_bdev.
> 
> 	if (++bdev->bd_fsfreeze_count > 1)
> 		goto done;
> 
> So you're freezing the device without really freezing the superblock.
> Seems like dangerous behavior to me. The new sb will only be frozen if
> a second thaw is done, which gets them back in sync. I suppose we could
> say this is acceptable loss, and your number of thaws should match your
> freezes, and if they don't: user error. Still, it seems like we should do
> something about it, like refuse to mount a frozen device. Perhaps it already
> does that; I'll need to do some research.

After some experiments, I've determined that my fears about the count are unfounded.
Consider my patch withdrawn. Sorry for the noise.

Bob Peterson


      reply	other threads:[~2021-01-08 15:00 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-24  4:49 [PATCH] fs: Fix freeze_bdev()/thaw_bdev() accounting of bd_fsfreeze_sb Satya Tangirala
2021-01-04 21:58 ` Darrick J. Wong
2021-01-05  7:50 ` Christoph Hellwig
2021-01-07 16:20 ` Christoph Hellwig
2021-01-07 16:26   ` Bob Peterson
2021-01-07 16:26   ` Jens Axboe
2021-01-07 16:27   ` Bob Peterson
2021-01-07 18:20     ` [fs PATCH] fs: fix freeze count problem in freeze_bdev Bob Peterson
2021-01-07 23:08     ` [PATCH] fs: Fix freeze_bdev()/thaw_bdev() accounting of bd_fsfreeze_sb Satya Tangirala
2021-01-08  9:36       ` Christoph Hellwig
2021-01-08 13:17       ` Bob Peterson
2021-01-08 14:58         ` Bob Peterson [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=1568673558.43563383.1610117926358.JavaMail.zimbra@redhat.com \
    --to=rpeterso@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=hch@lst.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=satyat@google.com \
    --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.