Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

F_OPEN_DIRECT_IO allow mmap #870

Merged
merged 4 commits into from
Jan 10, 2024
Merged

F_OPEN_DIRECT_IO allow mmap #870

merged 4 commits into from
Jan 10, 2024

Conversation

bsbernd
Copy link
Collaborator

@bsbernd bsbernd commented Dec 4, 2023

No description provided.

@bsbernd
Copy link
Collaborator Author

bsbernd commented Dec 4, 2023

@amir73il I guess that is what you also have? @szmi I'm not entire sure what to do about the the pending flag rename.

@szmi
Copy link
Contributor

szmi commented Dec 4, 2023

See git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git#for-next

@amir73il
Copy link
Contributor

amir73il commented Dec 4, 2023

@amir73il I guess that is what you also have? @szmi I'm not entire sure what to do about the the pending flag rename.

I just had a local patch in my build to forcefully add the init flag

@bsbernd
Copy link
Collaborator Author

bsbernd commented Dec 5, 2023

Thanks @szmi, updated.

@bsbernd bsbernd closed this Dec 5, 2023
@bsbernd bsbernd reopened this Dec 5, 2023
@bsbernd bsbernd closed this Dec 6, 2023
@bsbernd bsbernd reopened this Dec 6, 2023
@bsbernd bsbernd mentioned this pull request Dec 6, 2023
Merged
@Nikratio
Copy link
Contributor

Is this ready for merging and supported by a released kernel?

bsbernd
bsbernd commented Dec 22, 2023
View reviewed changes
lib/fuse_lowlevel.c Outdated
@@ -2151,6 +2153,8 @@ void do_init(fuse_req_t req, fuse_ino_t nodeid, const void *inarg)
outargflags |= FUSE_EXPLICIT_INVAL_DATA;
if (se->conn.want & FUSE_CAP_SETXATTR_EXT)
outargflags |= FUSE_SETXATTR_EXT;
if (se->conn.want & FUSE_CAP_DIRECT_IO_ALLOW_MMAP)
outargflags |= FUSE_DIRECT_IO_RELAX;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, just see, I should rename it.

@bsbernd
Copy link
Collaborator Author

bsbernd commented Dec 22, 2023

@Nikratio in linux-6.7 the flag is called FUSE_DIRECT_IO_RELAX (but just because Miklos didn't have to send a pull request). fuse-next branch already has the rename (and given the flag was already released, also has a compat define). I already added in that rename here.

@bsbernd
Synchronize fuse_kernel.h from current linux master
8a261e6
@bsbernd
Add in rename to FUSE_DIRECT_IO_ALLOW_MMAP
aa40f46 
Add in the rename of FUSE_DIRECT_IO_RELAX to
FUSE_DIRECT_IO_ALLOW_MMAP.
@bsbernd
Add FUSE_CAP_DIRECT_IO_ALLOW_MMAP and use in passthrough_hp
fe389b8 
This is not called FUSE_CAP_DIRECT_IO_RELAX, as the kernel flag
FUSE_DIRECT_IO_RELAX is supposed to be renamed to
FUSE_DIRECT_IO_ALLOW_MMAP. The corresponding kernel patches just
did not land yet.
@bsbernd
Copy link
Collaborator Author

bsbernd commented Dec 24, 2023

Looks like there is a generic issue with this test

ERROR: fuse_lowlevel_notify_store() failed with No such file or directory (2)

Going to look into it later.

@bsbernd bsbernd closed this Dec 24, 2023
@bsbernd bsbernd reopened this Dec 24, 2023
@bsbernd bsbernd force-pushed the direct-io-allow-mmap branch 2 times, most recently from bcc0c95 to fe389b8 Compare December 26, 2023 11:15
@bsbernd
Copy link
Collaborator Author

bsbernd commented Dec 26, 2023

@Nikratio I created a different pull request for the notification test failure, I think easier to review and merge with separate pull requests.

@bsbernd
Copy link
Collaborator Author

bsbernd commented Jan 9, 2024

@Nikratio any reason to hold this back? linux-6.7 is released now, which also includes commit c55e0a55b165202f18cbc4a20650d2e1becd5507

commit c55e0a55b165202f18cbc4a20650d2e1becd5507
Author: Tyler Fanelli <tfanelli@redhat.com>
Date:   Tue Sep 19 22:40:00 2023 -0400

    fuse: Rename DIRECT_IO_RELAX to DIRECT_IO_ALLOW_MMAP
    
    Although DIRECT_IO_RELAX's initial usage is to allow shared mmap, its
    description indicates a purpose of reducing memory footprint. This
    may imply that it could be further used to relax other DIRECT_IO
    operations in the future.
    
    Replace it with a flag DIRECT_IO_ALLOW_MMAP which does only one thing,
    allow shared mmap of DIRECT_IO files while still bypassing the cache
    on regular reads and writes.
    
    [Miklos] Also Keep DIRECT_IO_RELAX definition for backward compatibility.

Any reasons not to merge this pull request?

@Nikratio
Copy link
Contributor

Apologies. No reason other than lack of time for libfuse on my part. Unfortunately this lack of time also means that when I finally find some time I'm increasingly lacking context, so the reviews take even longer because I need to refresh my mind. I'll take your word for this one.

@Nikratio Nikratio merged commit 22741f5 into libfuse:master Jan 10, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@bsbernd @szmi @amir73il @Nikratio