Received-SPF: pass (google.com: domain of linux-kernel+bounces-15132-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Subject: Re: [PATCH RFC 00/17] ubifs: Add filesystem repair support
To: Richard Weinberger <richard@nod.at>
CC: david oberhollenzer <david.oberhollenzer@sigma-star.at>, Miquel Raynal
	<miquel.raynal@bootlin.com>, Sascha Hauer <s.hauer@pengutronix.de>, Tudor
 Ambarus <Tudor.Ambarus@linaro.org>, linux-kernel
	<linux-kernel@vger.kernel.org>, linux-mtd <linux-mtd@lists.infradead.org>
References: <20231228014112.2836317-1-chengzhihao1@huawei.com>
 <1145531757.175508.1703844362355.JavaMail.zimbra@nod.at>
 <13b259ca-b32f-a8d6-5e11-8bb38df72f5c@huawei.com>
 <642239519.177270.1703884138999.JavaMail.zimbra@nod.at>
 <a145fc68-9b0a-9794-48d2-b7ad79116833@huawei.com>
 <535616666.192239.1704228332389.JavaMail.zimbra@nod.at>
From: Zhihao Cheng <chengzhihao1@huawei.com>
Message-ID: <460eb02e-8937-282c-62c5-6ea606324b0e@huawei.com>
Date: Wed, 3 Jan 2024 11:18:34 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101
 Thunderbird/68.5.0
Precedence: bulk
MIME-Version: 1.0
In-Reply-To: <535616666.192239.1704228332389.JavaMail.zimbra@nod.at>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 8bit

在 2024/1/3 4:45, Richard Weinberger 写道:
> ----- Ursprüngliche Mail -----
>> Von: "chengzhihao1" <chengzhihao1@huawei.com>
>> I come up with another way to implement fsck.ubifs:
>>
>> There are three modes:
>>
>> 1. common mode(no options): Ask user whether to fix as long as a problem
>> is detected.
> 
> Makes sense.
> 
>> 2. safe mode(-a option): Auto repair as long as no data/files lost(eg.
>> nlink, isize, xattr_cnt, which can be corrected without dropping nodes),
>> otherwise returns error code.
> 
> Makes sense.
>   
>> 3. dangerous mode(-y option): Fix is always successful, unless
>> superblock is corrupted. There are 2 situations:
> 
> Please not use "-y". Usually "-y" stands for "answer yes to all questions".
> But selecting names for command line flags is currently my least concern.
>   
>>    a) TNC is valid: fsck will print which file is dropped and which
>> file's data is dropped
> 
> Sounds also good.
>   
>>    b) TNC is invalid: fsck will scan all nodes without referencing TNC,
>> same as this patchset does
> 
> I'd make this a distinct mode.
> It can be used to rebuild index and LEB property trees.
> This is basically a "drop trees and rebuild" mode.
> 

OK, then fsck will have four modes.

>>
>> Q1: How do you think of this method?
> 
> Sounds good so far.
>   
>> Q2: Mode 1, 2 and 3(a) depend on journal replaying, I found
>> xfs_repair[1] should be used after mounting/unmounting xfs (Let kernel
>> replay journal), if UBIFS does so, there is no need to copy recovery
>> subsystem into userspace, but user has to mount/unmount before doing
>> fsck. I found e2fsck has copied recovery code into userspace, so it can
>> do fsck without mounting/unmounting. If UBIFS does so, journal replaying
>> will update TNC and LPT, please reference Q3(1). Which method do you
>> suggest?
> 
> UBIFS is a little special regarding the journal.
> 
> 1. The journal is not an add-on like it is on many other file systems,
> you have to replay it to get a consistent file system.
> 2. Journal replay is also needed after a clean umount. The reason is that
> UBIFS does no commit at umount time.

I agree, there exists one situation that UBIFS replays journal even 
after clean umount.
     P1      ubifs_bgt      umount
   mkdir
          run_bg_commit
           c->cmt_state = COMMIT_RUNNING_BACKGROUND
           do_commit
            ubifs_log_start_commit(c, &new_ltail_lnum) // log start
            up_write(&c->commit_sem)
   touch
    ubifs_jnl_update // new buds added
                          cleanup_mnt
                           deactivate_super
                            fs->kill_sb
                             generic_shutdown_super
                              sync_filesystem
                               ubifs_sync_fs
                                ubifs_run_commit
                                 wait_for_commit // wait bg commit, 
'touch' won't be commited, it will be replayed in next mount

> 
> So IMHO you need to have journal replay code in your tool in any case.
> You can copy it from the kernel implementation and add more checks.
> While the kernel code also tries to be fast, fsck should be more paranoid.
> Ideally the outcome is a libubifs or such which can be derived from the
> kernel source while building mtd-utils.

All right, sounds like a huge copy work.

> 
>> Q3: If fsck drops or updates a node(eg. dentry lost inode, corrected
>> inode) in mode 1,2 and 3(a), TNC/LPT should be updated. There are two
>> ways updating TNC and LPT:
>>
>>    1) Like kernel does, which means that mark dirty TNC/LPT for
>> corresponding branches and do_commit(). It will copy much code into
>> userspace, eg. tnc.c, lpt.c, tnc_commit.c,
>> lpt_commit.c. I fear there exists risks. For example, there is no space
>> left for new index nodes, gc should be performed? If so, gc/lpt gc code
>> should be copied too.
>>
>>    2) Rebuild new TNC/LPT based on valid nodes. This way is simple, but
>> old good TNC could be corrupted, it means that powercut during fsck may
>> let UBIFS image must be repaired in mode 3(b) but it could be repaired
>> in mode 2\3(a) before invoking fsck.
>>
>> Which way is better?
> 
> Since you need to do a full journal replay anyway and power-cut awareness
> is one of your requirements, I fear the only option is 1). >
> Thanks,
> //richard
> .
>