All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: "Theodore Ts'o" <tytso@mit.edu>
To: bugzilla-daemon@kernel.org
Cc: linux-ext4@vger.kernel.org
Subject: Re: [Bug 218820] New: The empty file occupies incorrect blocks
Date: Wed, 8 May 2024 23:35:41 -0400	[thread overview]
Message-ID: <20240509033541.GB3620298@mit.edu> (raw)
In-Reply-To: <bug-218820-13602@https.bugzilla.kernel.org/>

On Wed, May 08, 2024 at 01:33:56PM +0000, bugzilla-daemon@kernel.org wrote:
> 
> The following is the triggering script:
> ```
> dd if=/dev/zero of=ext4-0.img bs=1M count=120
> mkfs.ext4 ext4-0.img
> g++ -static reproduce.c
> losetup /dev/loop0 ext4-0.img
> mkdir /root/mnt
> ./a.out
> stat /root/mnt/a
> ```
> 
> After run the script, you will get the following outputs:
> ```
>   File: /root/mnt/a
>   Size: 0               Blocks: 82         IO Block: 1024   regular empty file
> Device: 700h/1792d      Inode: 12          Links: 1
> Access: (0755/-rwxr-xr-x)  Uid: (    0/    root)   Gid: (    0/    root)
> Context: system_u:object_r:unlabeled_t:s0
> Access: 2024-05-08 11:47:48.000000000 +0000
> Modify: 2024-05-08 11:47:48.000000000 +0000
> Change: 2024-05-08 11:47:48.000000000 +0000
>  Birth: -
> ```

Thanks for the bug report.  What the reproducer script is doing is
opening a file for writing, and then remounting the file system to
disable delayhed allocations.  It then writes 40k to the file, and
then truncates the file, and close it.

The reproducer script leaves the file system mounted; if you unmount
the file system, the kernel will issue a warning message:

EXT4-fs (loop0): Inode 13 (0000000082f8ff6c): i_reserved_data_blocks (40) not cleared!

... and then if you examine the on-disk image, you'll see that the
i_blocks field is correct.


The root cause is that when the file is open, the address_operations
which is instaniated is one which is designed for delayed allocation.
So when 40k is written to the file, although we haven't allocated the
space yet, we *will* allocate the space, so we update in-memory
i_blocks to reflect the to-be-allocated disk space (although we don't
update the on-disk i_blocks until the allocation actually takes place,
so if we crash, the file system stays consistent).

However, some of the *other* logic is done based on whether the
delayed allocation flag is set in the struct super (as opposed to the
address_operations in the struct inode at the time that file is
opened).  This includes what happens when we truncate the file, where
in the nodelalloc case, the in-memory and on-disk i_blocks are in
sync, and when the blocks are delallocated, the i_blocks field is
dropped.  But since the blocks weren't actually allocated because the
file descriptor was in delalloc mode, there was nothing to
delallocate, so the in-memory i_blocks stayed elevanted.

There are a number ways of fixing this.  The simplest is to simply not
allow the delayed allocation mode to be changed on a remount.  Since
in the long term, once we fix some performance for some specialized
use case which has caused people to want to disable delayed
allocation, this is the one that probably makes the most sense.

The two other approaches involve a lot more complexity.  The first
alternate approach is to iterate over all open files, and resolve any
pending delayed allocations, and then update the address_operations to
use ext4_aops instead of ext4_da_aops, while avoiding the races
involving writes happening while we are trying to do the remount.
Yelch.

The second alternate approach is to treat the delayed allocation
status on a per-inode basis, with a per-inode flag indicating whether
delayed allocation is active, so that the truncate logic stays
consistent with the ext4_da_ops active on file descriptors associated
with the file.  But this gets super messy since subsequent file opens
on the inode also need to use the same delayed allocation mode until
the last file descriptor associated with the inode is closed.  Double
Yelch.

So preventing the "mount -o remount,nodelalloc" in the reproducer
program is how we'll address this issue.

					- Ted

  reply	other threads:[~2024-05-09  3:35 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-08 13:33 [Bug 218820] New: The empty file occupies incorrect blocks bugzilla-daemon
2024-05-09  3:35 ` Theodore Ts'o [this message]
2024-05-09  3:35 ` [Bug 218820] " bugzilla-daemon
2024-05-09  6:35 ` bugzilla-daemon

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=20240509033541.GB3620298@mit.edu \
    --to=tytso@mit.edu \
    --cc=bugzilla-daemon@kernel.org \
    --cc=linux-ext4@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 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.