[Ocfs2-devel] [PATCH 1/1] ocfs2: fix ocfs2_find_slot repeats alloc same slot issue

[heming.zhao at suse.com]

On 6/14/22 11:27, Joseph Qi wrote:
> 
> 
> On 6/14/22 10:59 AM, heming.zhao at suse.com wrote:
>> On 6/13/22 23:43, Junxiao Bi wrote:
>>>
>>>> 在 2022年6月13日，上午1:48，heming.zhao at suse.com 写道：
>>>>
>>>> On 6/13/22 16:21, Joseph Qi wrote:
>>>>>> On 6/13/22 3:59 PM, heming.zhao at suse.com wrote:
>>>>>> On 6/12/22 22:16, Joseph Qi wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Why can't use local mount? I don't remember if we discuss about this.
>>>>>>>
>>>>>> Sorry, I can't follow your question.
>>>>>> Do you mean why revert commit 912f655d78c5?
>>>>>>
>>>>>> or you are interest with the feature local mount?
>>>>>> the local mount is created by mkfs.ocfs2, which can't be converted to clustered.
>>>>>> see mkfs.ocfs2(8) '-M' option.
>>>>>>
>>>>> What Junxiao's main concern is data corruption, so I'm afraid we have to
>>>>> introduce an ondisk feature bit to prevent mixed nocluster and cluster
>>>>> mount, similar to local mount.
>>>>
>>>> this patch defined two new variants/flags:
>>>> #define OCFS2_SLOTMAP_CLUSTER   1
>>>> #define OCFS2_SLOTMAP_NOCLUSTER 2
>>>>
>>>> (I expect) Under OCFS2_SLOTMAP_CLUSTER, for clustered mount mode and for compatibility,
>>>> anything doesn't need to be changed.
>>>>
>>>> OCFS2_SLOTMAP_NOCLUSTER will introduce a new value for slotmap area.
>>>> this new value only take effect after a successfully nocluster mount.
>>>> (pls fix me), existed kernel/user space code don't do any special handle for
>>>> noclustered mount mode in slotmap area. So the new value is also compatibility.
>>>>
>>>> And the patch can also prevent mixed mount, the related code is in ocfs2_find_slot().
>>>> code logic:
>>>> - noclustered mount condition: slotmap is empty or already mounted with noclustered
>>>> - clustered mount condition: slotmap is empty or already mounted with clustered.
>>>> - all other conditions will be denied.
>>> Finding slot required reading slot map and then  update slot map. It is not atomic , you can’t prevent mixed mount until you have a cluster lock.
>>
>> Could I say your mentioned use case is invalid.
>> I believe all (yes, *all*) the ocfs2 users use this fs under cluster mode in
>> their product env.
>> The nocluster mount is only used on maintained env (eg. backup, fsck).
>>
>> We only concern two ways:
>> 1. user forgets to unmount (eg crash) before using another mount mode.
>> 2. when ocfs2 volume is working, which should deny volume is mounted by another mode.
>>     this may happened user mistakenly runs command or script.
>>
>> Base on above 1 & 2, Junxiao above mentioned use case only happens on one scenario:
>> the volume doesn't be mounted by any node, then user mistakenly mounts volume
>> with [no]cluster mode at same time. I believe this use case is invalid. And I guess
>> gfs2 may also has the same issue.
>>
>> With this patch, ocfs2 has more sanity check ability than other fs, eg: xfs, ext4.
>> SUSE HA stack with corosync+pacemaker also supports running xfs/ext4 with A/P mode.
>> The xfs/ext4 never have detecting mount status code, Junxiao mentioned mixed mount
>> can also happens on these fs. How do xfs/ext4/HA maintainers handle it? Under these
>> fs mounting behavior, these fields maintainers also treat the mixed mount as
>> invalid use case and ignore handle it.
> 
> Ext4 has a feature mmp (multiple mount protection) to do this.
> More details please refer to:
> https://ext4.wiki.kernel.org/index.php/Ext4_Disk_Layout
> 

Hi Joseph,

I checked mmp feature related code, it can help ext4 to avoid multiple mount issue.

 From my code reading:
- the mmp feature did mmp check when mouting.
- the mmp feature will start a kernel thread (kmmpd) to monitor mount status.
- kmmpd does detecting underlying dev IOs task.

We could re-use mmp feature idea for ocfs2, do you plan to add it?
If yes, I could take this feature porting job.

Thanks,
Heming