Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] fs: i_version mntopt gets visible through /proc/mounts
To:     "J. Bruce Fields" <bfields@fieldses.org>
Cc:     "Darrick J. Wong" <darrick.wong@oracle.com>,
        Christoph Hellwig <hch@infradead.org>,
        Masayoshi Mizuma <msys.mizuma@gmail.com>,
        Theodore Ts'o <tytso@mit.edu>,
        Andreas Dilger <adilger.kernel@dilger.ca>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>,
        linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        linux-xfs <linux-xfs@vger.kernel.org>
References: <20200616202123.12656-1-msys.mizuma@gmail.com>
 <20200617080314.GA7147@infradead.org> <20200617155836.GD13815@fieldses.org>
 <24692989-2ee0-3dcc-16d8-aa436114f5fb@sandeen.net>
 <20200617172456.GP11245@magnolia>
 <8f0df756-4f71-9d96-7a52-45bf51482556@sandeen.net>
 <20200617181816.GA18315@fieldses.org>
From:   Eric Sandeen <sandeen@sandeen.net>
Autocrypt: addr=sandeen@sandeen.net; prefer-encrypt=mutual; keydata=
 mQINBE6x99QBEADMR+yNFBc1Y5avoUhzI/sdR9ANwznsNpiCtZlaO4pIWvqQJCjBzp96cpCs
 nQZV32nqJBYnDpBDITBqTa/EF+IrHx8gKq8TaSBLHUq2ju2gJJLfBoL7V3807PQcI18YzkF+
 WL05ODFQ2cemDhx5uLghHEeOxuGj+1AI+kh/FCzMedHc6k87Yu2ZuaWF+Gh1W2ix6hikRJmQ
 vj5BEeAx7xKkyBhzdbNIbbjV/iGi9b26B/dNcyd5w2My2gxMtxaiP7q5b6GM2rsQklHP8FtW
 ZiYO7jsg/qIppR1C6Zr5jK1GQlMUIclYFeBbKggJ9mSwXJH7MIftilGQ8KDvNuV5AbkronGC
 sEEHj2khs7GfVv4pmUUHf1MRIvV0x3WJkpmhuZaYg8AdJlyGKgp+TQ7B+wCjNTdVqMI1vDk2
 BS6Rg851ay7AypbCPx2w4d8jIkQEgNjACHVDU89PNKAjScK1aTnW+HNUqg9BliCvuX5g4z2j
 gJBs57loTWAGe2Ve3cMy3VoQ40Wt3yKK0Eno8jfgzgb48wyycINZgnseMRhxc2c8hd51tftK
 LKhPj4c7uqjnBjrgOVaVBupGUmvLiePlnW56zJZ51BR5igWnILeOJ1ZIcf7KsaHyE6B1mG+X
 dmYtjDhjf3NAcoBWJuj8euxMB6TcQN2MrSXy5wSKaw40evooGwARAQABtCVFcmljIFIuIFNh
 bmRlZW4gPHNhbmRlZW5Ac2FuZGVlbi5uZXQ+iQI7BBMBAgAlAhsDBgsJCAcDAgYVCAIJCgsE
 FgIDAQIeAQIXgAUCUzMzbAIZAQAKCRAgrhaS4T3e4Fr7D/wO+fenqVvHjq21SCjDCrt8HdVj
 aJ28B1SqSU2toxyg5I160GllAxEHpLFGdbFAhQfBtnmlY9eMjwmJb0sCIrkrB6XNPSPA/B2B
 UPISh0z2odJv35/euJF71qIFgWzp2czJHkHWwVZaZpMWWNvsLIroXoR+uA9c2V1hQFVAJZyk
 EE4xzfm1+oVtjIC12B9tTCuS00pY3AUy21yzNowT6SSk7HAzmtG/PJ/uSB5wEkwldB6jVs2A
 sjOg1wMwVvh/JHilsQg4HSmDfObmZj1d0RWlMWcUE7csRnCE0ZWBMp/ttTn+oosioGa09HAS
 9jAnauznmYg43oQ5Akd8iQRxz5I58F/+JsdKvWiyrPDfYZtFS+UIgWD7x+mHBZ53Qjazszox
 gjwO9ehZpwUQxBm4I0lPDAKw3HJA+GwwiubTSlq5PS3P7QoCjaV8llH1bNFZMz2o8wPANiDx
 5FHgpRVgwLHakoCU1Gc+LXHXBzDXt7Cj02WYHdFzMm2hXaslRdhNGowLo1SXZFXa41KGTlNe
 4di53y9CK5ynV0z+YUa+5LR6RdHrHtgywdKnjeWdqhoVpsWIeORtwWGX8evNOiKJ7j0RsHha
 WrePTubr5nuYTDsQqgc2r4aBIOpeSRR2brlT/UE3wGgy9LY78L4EwPR0MzzecfE1Ws60iSqw
 Pu3vhb7h3bkCDQROsffUARAA0DrUifTrXQzqxO8aiQOC5p9Tz25Np/Tfpv1rofOwL8VPBMvJ
 X4P5l1V2yd70MZRUVgjmCydEyxLJ6G2YyHO2IZTEajUY0Up+b3ErOpLpZwhvgWatjifpj6bB
 SKuDXeThqFdkphF5kAmgfVAIkan5SxWK3+S0V2F/oxstIViBhMhDwI6XsRlnVBoLLYcEilxA
 2FlRUS7MOZGmRJkRtdGD5koVZSM6xVZQSmfEBaYQ/WJBGJQdPy94nnlAVn3lH3+N7pXvNUuC
 GV+t4YUt3tLcRuIpYBCOWlc7bpgeCps5Xa0dIZgJ8Louu6OBJ5vVXjPxTlkFdT0S0/uerCG5
 1u8p6sGRLnUeAUGkQfIUqGUjW2rHaXgWNvzOV6i3tf9YaiXKl3avFaNW1kKBs0T5M1cnlWZU
 Utl6k04lz5OjoNY9J/bGyV3DSlkblXRMK87iLYQSrcV6cFz9PRl4vW1LGff3xRQHngeN5fPx
 ze8X5NE3hb+SSwyMSEqJxhVTXJVfQWWW0dQxP7HNwqmOWYF/6m+1gK/Y2gY3jAQnsWTru4RV
 TZGnKwEPmOCpSUvsTRXsVHgsWJ70qd0yOSjWuiv4b8vmD3+QFgyvCBxPMdP3xsxN5etheLMO
 gRwWpLn6yNFq/xtgs+ECgG+gR78yXQyA7iCs5tFs2OrMqV5juSMGmn0kxJUAEQEAAYkCHwQY
 AQIACQUCTrH31AIbDAAKCRAgrhaS4T3e4BKwD/0ZOOmUNOZCSOLAMjZx3mtYtjYgfUNKi0ki
 YPveGoRWTqbis8UitPtNrG4XxgzLOijSdOEzQwkdOIp/QnZhGNssMejCnsluK0GQd+RkFVWN
 mcQT78hBeGcnEMAXZKq7bkIKzvc06GFmkMbX/gAl6DiNGv0UNAX+5FYh+ucCJZSyAp3sA+9/
 LKjxnTedX0aygXA6rkpX0Y0FvN/9dfm47+LGq7WAqBOyYTU3E6/+Z72bZoG/cG7ANLxcPool
 LOrU43oqFnD8QwcN56y4VfFj3/jDF2MX3xu4v2OjglVjMEYHTCxP3mpxesGHuqOit/FR+mF0
 MP9JGfj6x+bj/9JMBtCW1bY/aPeMdPGTJvXjGtOVYblGZrSjXRn5++Uuy36CvkcrjuziSDG+
 JEexGxczWwN4mrOQWhMT5Jyb+18CO+CWxJfHaYXiLEW7dI1AynL4jjn4W0MSiXpWDUw+fsBO
 Pk6ah10C4+R1Jc7dyUsKksMfvvhRX1hTIXhth85H16706bneTayZBhlZ/hK18uqTX+s0onG/
 m1F3vYvdlE4p2ts1mmixMF7KajN9/E5RQtiSArvKTbfsB6Two4MthIuLuf+M0mI4gPl9SPlf
 fWCYVPhaU9o83y1KFbD/+lh1pjP7bEu/YudBvz7F2Myjh4/9GUAijrCTNeDTDAgvIJDjXuLX pA==
Message-ID: <4cbb5cbe-feb4-2166-0634-29041a41a8dc@sandeen.net>
Date:   Wed, 17 Jun 2020 13:28:11 -0500
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:68.0)
 Gecko/20100101 Thunderbird/68.9.0
MIME-Version: 1.0
In-Reply-To: <20200617181816.GA18315@fieldses.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-ext4-owner@vger.kernel.org
Precedence: bulk


On 6/17/20 1:18 PM, J. Bruce Fields wrote:
> On Wed, Jun 17, 2020 at 12:55:24PM -0500, Eric Sandeen wrote:
>> On 6/17/20 12:24 PM, Darrick J. Wong wrote:
>>> On Wed, Jun 17, 2020 at 12:14:28PM -0500, Eric Sandeen wrote:
>>>>
>>>>
>>>> On 6/17/20 10:58 AM, J. Bruce Fields wrote:
>>>>> On Wed, Jun 17, 2020 at 01:03:14AM -0700, Christoph Hellwig wrote:
>>>>>> On Tue, Jun 16, 2020 at 04:21:23PM -0400, Masayoshi Mizuma wrote:
>>>>>>> From: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>
>>>>>>>
>>>>>>> /proc/mounts doesn't show 'i_version' even if iversion
>>>>>>> mount option is set to XFS.
>>>>>>>
>>>>>>> iversion mount option is a VFS option, not ext4 specific option.
>>>>>>> Move the handler to show_sb_opts() so that /proc/mounts can show
>>>>>>> 'i_version' on not only ext4 but also the other filesystem.
>>>>>>
>>>>>> SB_I_VERSION is a kernel internal flag.  XFS doesn't have an i_version
>>>>>> mount option.
>>>>>
>>>>> It probably *should* be a kernel internal flag, but it seems to work as
>>>>> a mount option too.
>>>>
>>>> Not on XFS AFAICT:
>>>>
>>>> [600280.685810] xfs: Unknown parameter 'i_version'
>>>
>>> Yeah, because the mount option is 'iversion', not 'i_version'.  Even if
>>
>> unless you're ext4:
>>
>> {Opt_i_version, "i_version"},
>>
>> ok "iversion" is what mount(8) takes and translates into MS_I_VERSION (thanks Darrick)
>>
>> # strace -vv -emount mount -oloop,iversion fsfile mnt
>> mount("/dev/loop0", "/tmp/mnt", "xfs", MS_I_VERSION, NULL) = 0
>>
>> FWIW, mount actually seems to pass what it finds in /proc/mounts back in on remount for ext4:
>>
>> # strace -vv -emount mount -o remount mnt
>> mount("/dev/loop0", "/tmp/mnt", 0x55bfcbdca150, MS_REMOUNT|MS_RELATIME, "seclabel,i_version,data=ordered") = 0
>>
>> but it still looks unhandled on remount.  Perhaps if /proc/mounts exposed
>> "iversion" (not "i_version") then mount -o remount would DTRT.
> 
> I'd rather just eliminate the option, to the extent possible.
> 
> It was only ever a mount option since it caused a performance regression
> in some filesystems, but I *think* that was addressed by Jeff Layton's
> work (f02a9ad1f15d "fs: handle inode->i_version more efficiently").
> 
> XFS in particular is just using this flag to tell knfsd that it should
> use i_version.  I don't think it was really intended for userspace to be
> able to turn this off.

but mount(8) has already exposed this interface:

       iversion
              Every time the inode is modified, the i_version field will be incremented.

       noiversion
              Do not increment the i_version inode field.

so now what?

FWIW, exporting "iversion" in /proc/mounts will 

1) tell us whether the SB_I_VERSION is or is not in fact set on the fs, and
2) will make remount DTRT because mount(8) will properly parse it and send it
   back in via the "flags" var during remount.

# mount -o loop fsfile mnt
# grep mnt /proc/mounts 
/dev/loop0 /tmp/mnt xfs rw,iversion,seclabel,relatime,attr2,inode64,logbufs=8,logbsize=32k,noquota 0 0

# strace -vv -emount mount -o remount mnt
mount("/dev/loop0", "/tmp/mnt", 0x55c11d0e3150, MS_REMOUNT|MS_RELATIME|MS_I_VERSION, "seclabel,attr2,inode64,logbufs=8"...) = 0

# grep mnt /proc/mounts 
/dev/loop0 /tmp/mnt xfs rw,iversion,seclabel,relatime,attr2,inode64,logbufs=8,logbsize=32k,noquota 0 0

ext4 i_version will just map back to iversion:

# mount -o i_version,loop fsfile mnt
# grep mnt /proc/mounts
/dev/loop0 /tmp/mnt ext4 rw,iversion,seclabel,relatime 0 0
# mount -o remount mnt
# grep mnt /proc/mounts
/dev/loop0 /tmp/mnt ext4 rw,iversion,seclabel,relatime 0 0

One wrinkle is that xfs will not honor "noiversion" currently but that's a different question.

-Eric