public inbox for linux-mtd@lists.infradead.org
 help / color / mirror / Atom feed
From: Zhihao Cheng <chengzhihao1@huawei.com>
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>
Subject: Re: [PATCH RFC 00/17] ubifs: Add filesystem repair support
Date: Wed, 3 Jan 2024 11:18:34 +0800	[thread overview]
Message-ID: <460eb02e-8937-282c-62c5-6ea606324b0e@huawei.com> (raw)
In-Reply-To: <535616666.192239.1704228332389.JavaMail.zimbra@nod.at>

在 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
> .
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

  reply	other threads:[~2024-01-03  3:19 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-28  1:40 [PATCH RFC 00/17] ubifs: Add filesystem repair support Zhihao Cheng
2023-12-28  1:40 ` [PATCH RFC 01/17] ubifs: repair: Load filesystem info from volume Zhihao Cheng
2023-12-28  1:40 ` [PATCH RFC 02/17] ubifs: repair: Scan nodes " Zhihao Cheng
2023-12-28  1:40 ` [PATCH RFC 03/17] ubifs: repair: Remove deleted nodes from valid node tree Zhihao Cheng
2023-12-28  1:40 ` [PATCH RFC 04/17] ubifs: repair: Add valid nodes into file Zhihao Cheng
2023-12-28  1:41 ` [PATCH RFC 05/17] ubifs: repair: Filter invalid files Zhihao Cheng
2023-12-28  1:41 ` [PATCH RFC 06/17] ubifs: repair: Extract reachable directory entries tree Zhihao Cheng
2023-12-28  1:41 ` [PATCH RFC 07/17] ubifs: repair: Check and correct files' information Zhihao Cheng
2023-12-28  1:41 ` [PATCH RFC 08/17] ubifs: repair: Record used LEBs Zhihao Cheng
2023-12-28  1:41 ` [PATCH RFC 09/17] ubifs: repair: Re-write data Zhihao Cheng
2023-12-28  1:41 ` [PATCH RFC 10/17] ubifs: repair: Create new root dir if there are no scanned files Zhihao Cheng
2023-12-28  1:41 ` [PATCH RFC 11/17] ubifs: repair: Build TNC Zhihao Cheng
2023-12-28  1:41 ` [PATCH RFC 12/17] ubifs: Extract a helper function to create lpt Zhihao Cheng
2023-12-28  1:41 ` [PATCH RFC 13/17] ubifs: repair: Build LPT Zhihao Cheng
2023-12-28  1:41 ` [PATCH RFC 14/17] ubifs: repair: Clean up log and orphan area Zhihao Cheng
2023-12-28  1:41 ` [PATCH RFC 15/17] ubifs: repair: Write master node Zhihao Cheng
2023-12-28  1:41 ` [PATCH RFC 16/17] ubifs: Enable ubifs_repair in '/sys/kernel/debug/ubifs/repair_fs' Zhihao Cheng
2023-12-28  1:41 ` [PATCH RFC 17/17] Documentation: ubifs: Add ubifs repair whitepaper Zhihao Cheng
2023-12-29 10:06 ` [PATCH RFC 00/17] ubifs: Add filesystem repair support Richard Weinberger
2023-12-29 13:09   ` Zhihao Cheng
2023-12-29 21:08     ` Richard Weinberger
2024-01-02 10:08       ` Zhihao Cheng
2024-01-02 20:45         ` Richard Weinberger
2024-01-03  3:18           ` Zhihao Cheng [this message]
2024-01-03 12:44             ` Zhihao Cheng
2024-01-03 12:55               ` Richard Weinberger
2024-01-03 13:33             ` Richard Weinberger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=460eb02e-8937-282c-62c5-6ea606324b0e@huawei.com \
    --to=chengzhihao1@huawei.com \
    --cc=Tudor.Ambarus@linaro.org \
    --cc=david.oberhollenzer@sigma-star.at \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=miquel.raynal@bootlin.com \
    --cc=richard@nod.at \
    --cc=s.hauer@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox