From mboxrd@z Thu Jan 1 00:00:00 1970 From: Al Viro Subject: Re: [PATCH] Introduce freeze_super and thaw_super for the fsfreeze ioctl Date: Tue, 23 Mar 2010 14:28:44 +0000 Message-ID: <20100323142843.GG30031@ZenIV.linux.org.uk> References: <20100323142200.GA2381@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, chris.mason@oracle.com, hch@lst.de To: Josef Bacik Return-path: Content-Disposition: inline In-Reply-To: <20100323142200.GA2381@localhost.localdomain> Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org On Tue, Mar 23, 2010 at 10:22:00AM -0400, Josef Bacik wrote: > Currently the way we do freezing is by passing sb>s_bdev to freeze_bdev and then > letting it do all the work. But freezing is more of an fs thing, and doesn't > really have much to do with the bdev at all, all the work gets done with the > super. In btrfs we do not populate s_bdev, since we can have multiple bdev's > for one fs and setting s_bdev makes removing devices from a pool kind of tricky. > This means that freezing a btrfs filesystem fails, which causes us to corrupt > with things like tux-on-ice which use the fsfreeze mechanism. So instead of > populating sb->s_bdev with a random bdev in our pool, I've broken the actual fs > freezing stuff into freeze_super and thaw_super. These just take the > super_block that we're freezing and does the appropriate work. It's basically > just copy and pasted from freeze_bdev. I've then converted freeze_bdev over to > use the new super helpers. I've tested this with ext4 and btrfs and verified > everything continues to work the same as before. > > The only new gotcha is multiple calls to the fsfreeze ioctl will return EBUSY if > the fs is already frozen. I thought this was a better solution than adding a > freeze counter to the super_block, but if everybody hates this idea I'm open to > suggestions. Thanks, Locking is all wrong there. We don't need to worry about umount; we *already* have an active reference. And leaving a kernel object with semaphore held when ioctl returns is completely wrong.