All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Sterba <dsterba@suse.cz>
To: Qu Wenruo <quwenruo.btrfs@gmx.com>
Cc: dsterba@suse.cz, Qu Wenruo <wqu@suse.de>,
	Qu Wenruo <wqu@suse.com>,
	linux-btrfs@vger.kernel.org, Nikolay Borisov <nborisov@suse.com>
Subject: Re: [PATCh v2 8/9] btrfs: tree-checker: Verify inode item
Date: Thu, 28 Mar 2019 15:25:44 +0100	[thread overview]
Message-ID: <20190328142544.GJ29086@twin.jikos.cz> (raw)
In-Reply-To: <e5de25af-6cec-fcaf-d3a6-5445ec0f45a5@gmx.com>

On Thu, Mar 28, 2019 at 10:13:19PM +0800, Qu Wenruo wrote:
> >>>> That means we have hard link for directories.
> >>>
> >>> Yes, hard links of directories are forbidden by VFS but that's not the
> >>> point here:
> >>>
> >>> https://btrfs.wiki.kernel.org/index.php/Project_ideas#Track_link_count_for_directories
> >>>
> >>> "The link count for directories is traditionally used to count the number
> >>> of subdirectores iff the link count is >= 2."
> >>
> >> Oh, got it.
> >>
> >> But for that case, it would be much better to go for things like nbytes
> >> or size, as by all means, those two members are less meaningful than
> >> nlinks for directory.
> >
> > size of a directory is currently 2 * sum of all names in the directory,
> > ie. both files and directories.
> 
> In fact that's not always the case for older kernels/progs.
> 
> IIRC it's in recent years that we enhanced btrfs-progs to detect and fix
> that.
> 
> > The nlink is used as an optimization
> > during traversal by existing tools (find), we can't simply change that
> > but would still like to update btrfs to provide support for that.
> 
> But current nlinks is persistent against all inodes. It's always the the
> number of INODE_REF the inode has.
> 
> While for size/nbytes, it doesn't make anything for directory inode at all.
> Kernel doesn't care, it's mostly btrfs-check check and fix.
> 
> Furthermore, only size is fixed to 2 * num_children.
> nbytes is only instructive and at least in lowmem mode, nbytes is only
> checked for alignment, even it's unaligned, lowmem check outputs warning
> only, doesn't count as an error.
> 
> So at least to me, directory nbytes is a better alternative, and it's
> back compatible.

The point here is to use nlinks of directories for the directory
traversal, the size/nbytes is not relevant here for the reasons you
state above.

I don't see how you think the proposed nbytes alternative would be used,
eg. by find.

  reply	other threads:[~2019-03-28 14:24 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-20  6:37 [PATCh v2 0/9] btrfs: tree-checker: More enhancement for fuzzed Qu Wenruo
2019-03-20  6:37 ` [PATCh v2 1/9] btrfs: Move btrfs_check_chunk_valid() to tree-check.[ch] and export it Qu Wenruo
2019-03-20 10:34   ` Johannes Thumshirn
2019-03-25 17:06   ` David Sterba
2019-03-25 23:02     ` Qu Wenruo
2019-03-26 14:34       ` David Sterba
2019-03-20  6:37 ` [PATCh v2 2/9] btrfs: tree-checker: Make chunk item checker more readable Qu Wenruo
2019-03-20 10:41   ` Johannes Thumshirn
2019-03-26 15:08     ` David Sterba
2019-03-20  6:37 ` [PATCh v2 3/9] btrfs: tree-checker: Make btrfs_check_chunk_valid() return EUCLEAN instead of EIO Qu Wenruo
2019-03-20 10:44   ` Johannes Thumshirn
2019-03-20  6:37 ` [PATCh v2 4/9] btrfs: tree-checker: Check chunk item at tree block read time Qu Wenruo
2019-03-20 10:56   ` Johannes Thumshirn
2019-03-20  6:37 ` [PATCh v2 5/9] btrfs: tree-checker: Verify dev item Qu Wenruo
2019-03-20 11:51   ` Johannes Thumshirn
2019-03-20 11:53     ` Qu Wenruo
2019-03-25 17:04       ` David Sterba
2019-04-06  1:07   ` Qu Wenruo
2019-03-20  6:37 ` [PATCh v2 6/9] btrfs: Check the first key and level for cached extent buffer Qu Wenruo
2019-03-20 12:02   ` Johannes Thumshirn
2019-03-20  6:37 ` [PATCh v2 7/9] btrfs: tree-checker: Enhance chunk checker to validate chunk profiler Qu Wenruo
2019-03-20 12:38   ` Johannes Thumshirn
2019-03-20  6:37 ` [PATCh v2 8/9] btrfs: tree-checker: Verify inode item Qu Wenruo
2019-03-20 13:27   ` Johannes Thumshirn
2019-03-25  4:27   ` Qu Wenruo
2019-03-26 16:02     ` David Sterba
2019-03-27  0:13       ` Qu Wenruo
2019-03-26 15:27   ` David Sterba
2019-03-28 13:38   ` David Sterba
2019-03-28 13:42     ` Qu Wenruo
2019-03-28 13:57       ` David Sterba
2019-03-28 14:00         ` Qu Wenruo
2019-03-28 14:07           ` David Sterba
2019-03-28 14:13             ` Qu Wenruo
2019-03-28 14:25               ` David Sterba [this message]
2019-03-28 23:49                 ` Qu Wenruo
2019-03-20  6:37 ` [PATCh v2 9/9] btrfs: inode: Verify inode mode to avoid NULL pointer dereference Qu Wenruo
2019-03-20 13:33   ` Johannes Thumshirn
2019-03-28 13:53   ` David Sterba
2019-03-28 13:58     ` Qu Wenruo
2019-03-28 14:02       ` David Sterba
2019-03-28 15:48 ` [PATCh v2 0/9] btrfs: tree-checker: More enhancement for fuzzed David Sterba

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=20190328142544.GJ29086@twin.jikos.cz \
    --to=dsterba@suse.cz \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=nborisov@suse.com \
    --cc=quwenruo.btrfs@gmx.com \
    --cc=wqu@suse.com \
    --cc=wqu@suse.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.