Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 8 Jan 2021 09:58:46 -0500 (EST)
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>
Message-ID: <1568673558.43563383.1610117926358.JavaMail.zimbra@redhat.com>
In-Reply-To: <879072186.43549344.1610111831181.JavaMail.zimbra@redhat.com>
References: <20201224044954.1349459-1-satyat@google.com> <20210107162000.GA2693@lst.de> <1137375419.42956970.1610036857271.JavaMail.zimbra@redhat.com> <X/eUd4iLxnl2nYRF@google.com> <879072186.43549344.1610111831181.JavaMail.zimbra@redhat.com>
Subject: Re: [PATCH] fs: Fix freeze_bdev()/thaw_bdev() accounting of
 bd_fsfreeze_sb
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Thread-Topic: Fix freeze_bdev()/thaw_bdev() accounting of bd_fsfreeze_sb
Thread-Index: Qh0Y8TOdr+3c+YdH/MUolaQzVBj28yl6kjYT
Precedence: bulk

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