linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: John Garry <john.g.garry@oracle.com>
To: Ojaswin Mujoo <ojaswin@linux.ibm.com>
Cc: linux-ext4@vger.kernel.org, Theodore Ts'o <tytso@mit.edu>,
	Ritesh Harjani <ritesh.list@gmail.com>,
	linux-kernel@vger.kernel.org,
	"Darrick J . Wong" <djwong@kernel.org>,
	linux-block@vger.kernel.org, linux-xfs@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, dchinner@redhat.com
Subject: Re: [RFC 0/7] ext4: Allocator changes for atomic write support with DIO
Date: Mon, 4 Dec 2023 14:44:36 +0000	[thread overview]
Message-ID: <de90e79b-83f2-428f-bac6-0754708aa4a8@oracle.com> (raw)
In-Reply-To: <ZW3WZ6prrdsPc55Z@li-bb2b2a4c-3307-11b2-a85c-8fa5c3a69313.ibm.com>

On 04/12/2023 13:38, Ojaswin Mujoo wrote:
>> So are we supposed to be doing atomic writes on unwritten ranges only in the
>> file to get the aligned allocations?
> If we do an atomic write on a hole, ext4 will give us an aligned extent
> provided the hole is big enough to accomodate it.
> 
> However, if we do an atomic write on a existing extent (written or
> unwritten) ext4 would check if it satisfies the alignment and length
> requirement and returns an error if it doesn't.

This seems a rather big drawback.

> Since we don't have cow
> like functionality afaik the only way we could let this kind of write go
> through is by punching the pre-existing extent which is not something we
> can directly do in the same write operation, hence we return error.

Well, as you prob saw, for XFS we were relying on forcing extent 
alignment, and not CoW (yet).

> 
>> I actually tried that, and I got a WARN triggered:
>>
>> # mkfs.ext4 /dev/sda
>> mke2fs 1.46.5 (30-Dec-2021)
>> Creating filesystem with 358400 1k blocks and 89760 inodes
>> Filesystem UUID: 7543a44b-2957-4ddc-9d4a-db3a5fd019c9
>> Superblock backups stored on blocks:
>>          8193, 24577, 40961, 57345, 73729, 204801, 221185
>>
>> Allocating group tables: done
>> Writing inode tables: done
>> Creating journal (8192 blocks): done
>> Writing superblocks and filesystem accounting information: done
>>
>> [   12.745889] mkfs.ext4 (150) used greatest stack depth: 13304 bytes left
>> # mount /dev/sda mnt
>> [   12.798804] EXT4-fs (sda): mounted filesystem
>> 7543a44b-2957-4ddc-9d4a-db3a5fd019c9 r/w with ordered data mode. Quota
>> mode: none.
>> # touch mnt/file
>> #
>> # /test-statx -a /root/mnt/file
>> statx(/root/mnt/file) = 0
>> dump_statx results=5fff
>>    Size: 0               Blocks: 0          IO Block: 1024    regular file
>> Device: 08:00           Inode: 12          Links: 1
>> Access: (0644/-rw-r--r--)  Uid:     0   Gid:     0
>> Access: 2023-12-04 10:27:40.002848720+0000
>> Modify: 2023-12-04 10:27:40.002848720+0000
>> Change: 2023-12-04 10:27:40.002848720+0000
>>   Birth: 2023-12-04 10:27:40.002848720+0000
>> stx_attributes_mask=0x703874
>>          STATX_ATTR_WRITE_ATOMIC set
>>          unit min: 1024
>>          uunit max: 524288
>> Attributes: 0000000000400000 (........ ........ ........ ........
>> ........ .?--.... ..---... .---.-..)
>> #
>>
>>
>>
>> looks ok so far, then write 4KB at offset 0:
>>
>> # /test-pwritev2 -a -d -p 0 -l 4096  /root/mnt/file
>> file=/root/mnt/file write_size=4096 offset=0 o_flags=0x4002 wr_flags=0x24

...

>> Please note that I tested on my own dev branch, which contains changes over
>> [1], but I expect it would not make a difference for this test.
> Hmm this should not ideally happen, can you please share your test
> script with me if possible?

It's doing nothing special, just RWF_ATOMIC flag is set for DIO write:

https://github.com/johnpgarry/linux/blob/236870d48ecb19c1cf89dc439e188182a0524cd4/samples/vfs/test-pwritev2.c

>>>
>>> Script to test using pwritev2() can be found here:
>>> https://urldefense.com/v3/__https://gist.github.com/OjaswinM/e67accee3cbb7832bd3f1a9543c01da9__;!!ACWV5N9M2RV99hQ!Lr0j4iDHrfXisXOGZ82HNPefBtVDDGe9zbLhey7rRDfPE7A_tsrrQ9Dw_4Ng_qS7xTGCZaEWBKtd6pqA_LIBfA$  
>> Please note that the posix_memalign() call in the program should PAGE align.
> Why do you say that? direct IO seems to be working when the userspace
> buffer is 512 byte aligned, am I missing something?

ah, sorry, if you just use 1x IOV vector then no special alignment are 
required, so ignore this. Indeed, I need to improve kernel checks for 
alignment anyway.

Thanks,
John


  reply	other threads:[~2023-12-04 14:49 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-30 13:53 [RFC 0/7] ext4: Allocator changes for atomic write support with DIO Ojaswin Mujoo
2023-11-30 13:53 ` [RFC 1/7] iomap: Don't fall back to buffered write if the write is atomic Ojaswin Mujoo
2023-11-30 21:10   ` Dave Chinner
2023-12-01 10:42     ` John Garry
2023-12-01 13:27       ` Matthew Wilcox
2023-12-01 19:06         ` John Garry
2023-12-01 22:07       ` Dave Chinner
2023-12-04  9:02         ` John Garry
2023-12-04 18:17           ` Darrick J. Wong
2023-12-04 18:34             ` John Garry
2023-12-07 12:43         ` John Garry
2023-11-30 13:53 ` [RFC 2/7] ext4: Factor out size and start prediction from ext4_mb_normalize_request() Ojaswin Mujoo
2023-11-30 13:53 ` [RFC 3/7] ext4: add aligned allocation support in mballoc Ojaswin Mujoo
2023-11-30 13:53 ` [RFC 4/7] ext4: allow inode preallocation for aligned alloc Ojaswin Mujoo
2023-11-30 13:53 ` [RFC 5/7] block: export blkdev_atomic_write_valid() and refactor api Ojaswin Mujoo
2023-12-01 10:47   ` John Garry
2023-12-11 10:57     ` Ojaswin Mujoo
2023-11-30 13:53 ` [RFC 6/7] ext4: Add aligned allocation support for atomic direct io Ojaswin Mujoo
2023-11-30 13:53 ` [RFC 7/7] ext4: Support atomic write for statx Ojaswin Mujoo
2023-12-04 10:36 ` [RFC 0/7] ext4: Allocator changes for atomic write support with DIO John Garry
2023-12-04 13:38   ` Ojaswin Mujoo
2023-12-04 14:44     ` John Garry [this message]
2023-12-11 10:54       ` Ojaswin Mujoo
2023-12-12  7:46         ` John Garry
2023-12-12 13:10           ` Christoph Hellwig
2023-12-12 15:16             ` Theodore Ts'o
2023-12-12 15:19               ` Christoph Hellwig
2023-12-12 16:10             ` John Garry
2023-12-13  5:59           ` Ojaswin Mujoo
2023-12-13  9:17             ` John Garry
2023-12-13  6:42         ` Ojaswin Mujoo
2023-12-13  9:20           ` John Garry

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=de90e79b-83f2-428f-bac6-0754708aa4a8@oracle.com \
    --to=john.g.garry@oracle.com \
    --cc=dchinner@redhat.com \
    --cc=djwong@kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=ojaswin@linux.ibm.com \
    --cc=ritesh.list@gmail.com \
    --cc=tytso@mit.edu \
    /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;
as well as URLs for NNTP newsgroup(s).