Re: [PATCH 0/2] fuse: Rename DIRECT_IO_{RELAX -> ALLOW_MMAP}

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Amir,

On 12/3/23 12:20, Amir Goldstein wrote:
On Sat, Dec 2, 2023 at 5:06 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote:

On Mon, Nov 6, 2023 at 4:08 PM Bernd Schubert
<bernd.schubert@xxxxxxxxxxx> wrote:

Hi Miklos,

On 9/20/23 10:15, Miklos Szeredi wrote:
On Wed, 20 Sept 2023 at 04:41, Tyler Fanelli <tfanelli@xxxxxxxxxx> wrote:

At the moment, FUSE_INIT's DIRECT_IO_RELAX flag only serves the purpose
of allowing shared mmap of files opened/created with DIRECT_IO enabled.
However, it leaves open the possibility of further relaxing the
DIRECT_IO restrictions (and in-effect, the cache coherency guarantees of
DIRECT_IO) in the future.

The DIRECT_IO_ALLOW_MMAP flag leaves no ambiguity of its purpose. It
only serves to allow shared mmap of DIRECT_IO files, while still
bypassing the cache on regular reads and writes. The shared mmap is the
only loosening of the cache policy that can take place with the flag.
This removes some ambiguity and introduces a more stable flag to be used
in FUSE_INIT. Furthermore, we can document that to allow shared mmap'ing
of DIRECT_IO files, a user must enable DIRECT_IO_ALLOW_MMAP.

Tyler Fanelli (2):
    fs/fuse: Rename DIRECT_IO_RELAX to DIRECT_IO_ALLOW_MMAP
    docs/fuse-io: Document the usage of DIRECT_IO_ALLOW_MMAP

Looks good.

Applied, thanks.  Will send the PR during this merge window, since the
rename could break stuff if already released.

I'm just porting back this feature to our internal fuse module and it
looks these rename patches have been forgotten?



Hi Miklos, Bernd,

I was looking at the DIRECT_IO_ALLOW_MMAP code and specifically at
commit b5a2a3a0b776 ("fuse: write back dirty pages before direct write in
direct_io_relax mode") and I was wondering - isn't dirty pages writeback
needed *before* invalidate_inode_pages2() in fuse_file_mmap() for
direct_io_allow_mmap case?

For FUSE_PASSTHROUGH, I am going to need to call fuse_vma_close()
for munmap of files also in direct-io mode [1], so I was considering installing
fuse_file_vm_ops for the FOPEN_DIRECT_IO case, same as caching case,
and regardless of direct_io_allow_mmap.

I was asking myself if there was a good reason why fuse_page_mkwrite()/
fuse_wait_on_page_writeback()/fuse_vma_close()/write_inode_now()
should NOT be called for the FOPEN_DIRECT_IO case regardless of
direct_io_allow_mmap?


Before trying to make changes to fuse_file_mmap() I tried to test
DIRECT_IO_RELAX - I enabled it in libfuse and ran fstest with
passthrough_hp --direct-io.

The test generic/095 - "Concurrent mixed I/O (buffer I/O, aiodio, mmap, splice)
on the same files" blew up hitting BUG_ON(fi->writectr < 0) in
fuse_set_nowrite()

I am wondering how this code was tested?

I could not figure out the problem and how to fix it.
Please suggest a fix and let me know which adjustments are needed
if I want to use fuse_file_vm_ops for all mmap modes.

So fuse_set_nowrite() tests for inode_is_locked(), but that also succeeds for a shared lock. It gets late here (and I might miss something), but I think we have an issue with FOPEN_PARALLEL_DIRECT_WRITES. Assuming there would be plain O_DIRECT and mmap, the same issue might triggered? Hmm, well, so far plain O_DIRECT does not support FOPEN_PARALLEL_DIRECT_WRITES yet - the patches for that are still pending.


Will look into it in more detail in the morning.

Thanks,
Bernd


Thanks,
Amir.

generic/095 5s ...  [10:53:05][   61.185656] kernel BUG at fs/fuse/dir.c:1756!
[   61.186653] invalid opcode: 0000 [#1] PREEMPT SMP PTI
[   61.187447] CPU: 2 PID: 3599 Comm: fio Not tainted 6.6.0-xfstests #2025
[   61.188461] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009),
BIOS 1.15.0-1 04/01/2014
[   61.189529] RIP: 0010:fuse_set_nowrite+0x47/0xdd
[   61.190117] Code: 48 8b 87 e8 00 00 00 48 85 c0 75 02 0f 0b 48 8d
af 38 06 00 00 48 89 fb 48 89 ef e8 e8 2b 8f 00 8b 83 28 05 00 00 85
c0 79 02 <0f> 0b 05 00 00 00 80 48 89 ef 89 83 28 05 00 00 e8 86 30 8f
00 be
[   61.192497] RSP: 0018:ffffc9000313fc98 EFLAGS: 00010282
[   61.193109] RAX: 0000000080000001 RBX: ffff88800cfb21c0 RCX: ffffc9000313fc3c
[   61.193937] RDX: 0000000000000003 RSI: ffffffff827ce6be RDI: ffffffff828a86cd
[   61.194736] RBP: ffff88800cfb27f8 R08: 0000000e3ef2354a R09: 0000000000000000
[   61.195509] R10: ffffffff82b74f20 R11: 0000000000000002 R12: ffff888009bf1f00
[   61.196291] R13: ffffc9000313fe70 R14: 0000000000000002 R15: ffff88800cfb23f0
[   61.197069] FS:  00007fa089f64740(0000) GS:ffff88807da00000(0000)
knlGS:0000000000000000
[   61.198024] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   61.198701] CR2: 00007fa089f17fe0 CR3: 0000000009202001 CR4: 0000000000370ee0
[   61.199817] Call Trace:
[   61.200198]  <TASK>
[   61.200486]  ? __die_body+0x1b/0x59
[   61.200975]  ? die+0x35/0x4f
[   61.201379]  ? do_trap+0x7c/0xff
[   61.201828]  ? fuse_set_nowrite+0x47/0xdd
[   61.202303]  ? do_error_trap+0xbe/0xeb
[   61.202733]  ? fuse_set_nowrite+0x47/0xdd
[   61.203196]  ? fuse_set_nowrite+0x47/0xdd
[   61.203723]  ? exc_invalid_op+0x52/0x69
[   61.204202]  ? fuse_set_nowrite+0x47/0xdd
[   61.204720]  ? asm_exc_invalid_op+0x1a/0x20
[   61.205204]  ? fuse_set_nowrite+0x47/0xdd
[   61.205628]  ? fuse_set_nowrite+0x3d/0xdd
[   61.206061]  ? do_raw_spin_unlock+0x88/0x8f
[   61.206498]  ? _raw_spin_unlock+0x2d/0x43
[   61.206915]  ? fuse_range_is_writeback+0x71/0x84
[   61.207383]  fuse_sync_writes+0xf/0x19
[   61.207857]  fuse_direct_io+0x167/0x5bd
[   61.208375]  fuse_direct_write_iter+0xf0/0x146
[   61.208990]  vfs_write+0x11d/0x1c4
[   61.209458]  ksys_pwrite64+0x68/0x87
[   61.209959]  do_syscall_64+0x6e/0x88





[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux