From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dmitry Monakhov Subject: Re: [PATCH 3/3] vfs: fix filesystem_sync vs write race on rw=>ro remount v2 Date: Tue, 02 Mar 2010 18:38:52 +0300 Message-ID: <874okyd983.fsf@openvz.org> References: <1267230349-8192-1-git-send-email-dmonakhov@openvz.org> <1267230349-8192-2-git-send-email-dmonakhov@openvz.org> <1267230349-8192-3-git-send-email-dmonakhov@openvz.org> <20100302132911.GB4763@quack.suse.cz> <87zl2qddlg.fsf@openvz.org> <20100302140621.GC3829@quack.suse.cz> <87sk8idcnv.fsf@openvz.org> <20100302143527.GD3829@quack.suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-fsdevel@vger.kernel.org, viro@ZenIV.linux.org.uk To: Jan Kara Return-path: Received: from mail-bw0-f209.google.com ([209.85.218.209]:33089 "EHLO mail-bw0-f209.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751495Ab0CBPi5 (ORCPT ); Tue, 2 Mar 2010 10:38:57 -0500 Received: by bwz1 with SMTP id 1so264386bwz.21 for ; Tue, 02 Mar 2010 07:38:55 -0800 (PST) In-Reply-To: <20100302143527.GD3829@quack.suse.cz> (Jan Kara's message of "Tue, 2 Mar 2010 15:35:27 +0100") Sender: linux-fsdevel-owner@vger.kernel.org List-ID: Jan Kara writes: > On Tue 02-03-10 17:24:36, Dmitry Monakhov wrote: >> Jan Kara writes: >> >> > On Tue 02-03-10 17:04:26, Dmitry Monakhov wrote: >> >> Jan Kara writes: >> >> >> >> > On Sat 27-02-10 03:25:49, Dmitry Monakhov wrote: >> >> >> +/** >> >> >> + * Check whenever is it possible to remount given sb to readonly. >> >> >> + * @sb : super block in question >> >> >> + * >> >> >> + * Caller is responsible to set ST_REMOUNT_RO state before the call. >> >> >> + */ >> >> >> +int fs_may_remount_ro(struct super_block *sb) >> >> >> +{ >> >> >> + struct vfsmount *mnt; >> >> >> + int ret = 1; >> >> >> + spin_lock(&vfsmount_lock); >> >> >> + list_for_each_entry(mnt, &sb->s_vfsmount, mnt_sb_list) { >> >> >> + ret = !mnt_check_writers(mnt, 0); >> >> >> + if (ret) >> >> >> + break; >> >> >> + } >> >> >> + spin_unlock(&vfsmount_lock); >> >> >> + /* >> >> >> + * If new writer appears after we have checked all vfsmounts. >> >> >> + * Then ST_REMOUNT_RO bit will be cleared. >> >> >> + */ >> >> >> + if (!test_bit(ST_REMOUNT_RO, &sb->s_state)) >> >> >> + ret = 0; >> >> >> + return ret; >> >> >> +} >> >> > This misses the case when the superblock as unlinked-but-open files. >> >> > In such case we have to fail remount RO as well. The original >> >> > fs_may_remount_ro checks for that.. >> >> Since file is opened for write one of vfsmnt struct would have non >> >> zero write count. So -EBUSY will be returned from fs_may_remount_ro() >> > But file can be open for reading only... >> Ohh.. i see. what is the reason to fail RO remount due to unlinked >> files? Is this because not all filesystem has orphan list? > Yes, this is the main reason. > >> Shame on such FS. Seems that i have to also grab write count >> in unlink path if i_nlink becomes zero and drop on inode release. > Yes, I'd imagine some solution like this as well. I've take a look at the code. It appear not that easy as it looks from the first glance. It is not obvious how to store mnt on which write count was incremented during unlink. Also there is no dedicated mnt which is assessable from SB. We have s_root but don't know in which mnt this dentry belongs. So the only option is to introduce real super_block counter and use it for unlinked inodes tracking.