From: Dave Chinner <david@fromorbit.com>
To: Chandan Babu R <chandan.babu@oracle.com>
Cc: linux-xfs@vger.kernel.org, djwong@kernel.org
Subject: Re: [PATCH V8 15/19] xfs: Directory's data fork extent counter can never overflow
Date: Fri, 25 Mar 2022 09:14:06 +1100 [thread overview]
Message-ID: <20220324221406.GL1544202@dread.disaster.area> (raw)
In-Reply-To: <20220321051750.400056-16-chandan.babu@oracle.com>
On Mon, Mar 21, 2022 at 10:47:46AM +0530, Chandan Babu R wrote:
> The maximum file size that can be represented by the data fork extent counter
> in the worst case occurs when all extents are 1 block in length and each block
> is 1KB in size.
>
> With XFS_MAX_EXTCNT_DATA_FORK_SMALL representing maximum extent count and with
> 1KB sized blocks, a file can reach upto,
> (2^31) * 1KB = 2TB
>
> This is much larger than the theoretical maximum size of a directory
> i.e. 32GB * 3 = 96GB.
>
> Since a directory's inode can never overflow its data fork extent counter,
> this commit replaces checking the return value of
> xfs_iext_count_may_overflow() with calls to ASSERT(error == 0).
I'd really prefer that we don't add noise like this to a bunch of
call sites. If directories can't overflow the extent count in
normal operation, then why are we even calling
xfs_iext_count_may_overflow() in these paths? i.e. an overflow would
be a sign of an inode corruption, and we should have flagged that
long before we do an operation that might overflow the extent count.
So, really, I think you should document the directory size
constraints at the site where we define all the large extent count
values in xfs_format.h, remove the xfs_iext_count_may_overflow()
checks from the directory code and replace them with a simple inode
verifier check that we haven't got more than 100GB worth of
individual extents in the data fork for directory inodes....
Then all this directory specific "can't possibly overflow" overflow
checks can go away completely. The best code is no code :)
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
next prev parent reply other threads:[~2022-03-24 22:14 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-21 5:17 [PATCH V8 00/19] xfs: Extend per-inode extent counters Chandan Babu R
2022-03-21 5:17 ` [PATCH V8 01/19] xfs: Move extent count limits to xfs_format.h Chandan Babu R
2022-03-21 5:17 ` [PATCH V8 02/19] xfs: Define max extent length based on on-disk format definition Chandan Babu R
2022-03-21 5:17 ` [PATCH V8 03/19] xfs: Introduce xfs_iext_max_nextents() helper Chandan Babu R
2022-03-21 5:17 ` [PATCH V8 04/19] xfs: Use xfs_extnum_t instead of basic data types Chandan Babu R
2022-03-21 5:17 ` [PATCH V8 05/19] xfs: Introduce xfs_dfork_nextents() helper Chandan Babu R
2022-03-24 21:31 ` Dave Chinner
2022-03-21 5:17 ` [PATCH V8 06/19] xfs: Use basic types to define xfs_log_dinode's di_nextents and di_anextents Chandan Babu R
2022-03-21 5:17 ` [PATCH V8 07/19] xfs: Promote xfs_extnum_t and xfs_aextnum_t to 64 and 32-bits respectively Chandan Babu R
2022-03-24 21:33 ` Dave Chinner
2022-03-21 5:17 ` [PATCH V8 08/19] xfs: Introduce XFS_SB_FEAT_INCOMPAT_NREXT64 and associated per-fs feature bit Chandan Babu R
2022-03-24 21:37 ` Dave Chinner
2022-03-24 21:40 ` Darrick J. Wong
2022-03-25 6:01 ` Chandan Babu R
2022-03-21 5:17 ` [PATCH V8 09/19] xfs: Introduce XFS_FSOP_GEOM_FLAGS_NREXT64 Chandan Babu R
2022-03-21 5:17 ` [PATCH V8 10/19] xfs: Introduce XFS_DIFLAG2_NREXT64 and associated helpers Chandan Babu R
2022-03-24 21:38 ` Dave Chinner
2022-03-21 5:17 ` [PATCH V8 11/19] xfs: Use uint64_t to count maximum blocks that can be used by BMBT Chandan Babu R
2022-03-24 21:42 ` Dave Chinner
2022-03-25 6:01 ` Chandan Babu R
2022-03-21 5:17 ` [PATCH V8 12/19] xfs: Introduce macros to represent new maximum extent counts for data/attr forks Chandan Babu R
2022-03-21 5:17 ` [PATCH V8 13/19] xfs: Replace numbered inode recovery error messages with descriptive ones Chandan Babu R
2022-03-24 21:47 ` Dave Chinner
2022-03-21 5:17 ` [PATCH V8 14/19] xfs: Introduce per-inode 64-bit extent counters Chandan Babu R
2022-03-24 21:53 ` Dave Chinner
2022-03-25 6:02 ` Chandan Babu R
2022-03-21 5:17 ` [PATCH V8 15/19] xfs: Directory's data fork extent counter can never overflow Chandan Babu R
2022-03-24 22:14 ` Dave Chinner [this message]
2022-03-25 6:02 ` Chandan Babu R
2022-03-25 6:37 ` Chandan Babu R
2022-03-29 5:22 ` Chandan Babu R
2022-03-29 6:23 ` Dave Chinner
2022-03-30 3:43 ` Darrick J. Wong
2022-03-30 15:39 ` Chandan Babu R
2022-03-30 15:52 ` Darrick J. Wong
2022-04-01 1:27 ` Dave Chinner
2022-04-01 7:46 ` Chandan Babu R
2022-03-21 5:17 ` [PATCH V8 16/19] xfs: Conditionally upgrade existing inodes to use large extent counters Chandan Babu R
2022-03-24 22:28 ` Dave Chinner
2022-03-25 6:02 ` Chandan Babu R
2022-03-21 5:17 ` [PATCH V8 17/19] xfs: Decouple XFS_IBULK flags from XFS_IWALK flags Chandan Babu R
2022-03-24 22:28 ` Dave Chinner
2022-03-21 5:17 ` [PATCH V8 18/19] xfs: Enable bulkstat ioctl to support 64-bit per-inode extent counters Chandan Babu R
2022-03-24 22:33 ` Dave Chinner
2022-03-21 5:17 ` [PATCH V8 19/19] xfs: Add XFS_SB_FEAT_INCOMPAT_NREXT64 to the list of supported flags Chandan Babu R
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=20220324221406.GL1544202@dread.disaster.area \
--to=david@fromorbit.com \
--cc=chandan.babu@oracle.com \
--cc=djwong@kernel.org \
--cc=linux-xfs@vger.kernel.org \
/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