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.

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

Added support for setting max-pages via config #347

Closed
wants to merge 1 commit into from

Conversation

darthShadow
Copy link

Added support for setting max-pages via config. Also increased the max write from 128k to 1M.

Signed-off-by: Anagh Kumar Baranwal 6824881+darthShadow@users.noreply.github.com

Signed-off-by: Anagh Kumar Baranwal <6824881+darthShadow@users.noreply.github.com>
@hanwen
Copy link
Owner

hanwen commented Feb 18, 2020

This needs a tests. Have a look at fs/directio_test.go for some inspiration.

You can do a large write by calling syscall.Write on 1 MB slice. On the FUSE side, the test should check that the requests are coming in as 1 MB in the Write call.

@rfjakob
Copy link
Contributor

rfjakob commented Jan 4, 2022

With this PR plus the change below against loopback I was able to get 1 MiB writes [1] but not 1 MiB reads [2]. @darthShadow did you manage to get large reads (how?)?

diff --git a/example/loopback/main.go b/example/loopback/main.go
index 4204c54..b2d5abe 100644
--- a/example/loopback/main.go
+++ b/example/loopback/main.go
@@ -104,6 +104,8 @@ func main() {
        opts.MountOptions.Options = append(opts.MountOptions.Options, "fsname="+orig)
        // Second column in "df -T" will be shown as "fuse." + Name
        opts.MountOptions.Name = "loopback"
+       opts.MountOptions.MaxWrite = 1024 * 1024
+       opts.MountOptions.MaxPages = 256
        // Leave file permissions on "000" files as-is
        opts.NullPermissions = true
        // Enable diagnostics logging

[1]:

dd if=/dev/zero bs=1M count=1 of=mnt/foo

20:23:19.336027 rx 38: WRITE i5004682 {Fh 1 [0 +1048576)  L 0 WRONLY,0x8000}  1048576b
20:23:19.336635 tx 38:     OK

[2]:

dd if=mnt/foo of=/dev/null bs=1M count=1

20:23:19.338086 rx 50: READ i5004682 {Fh 1 [0 +131072)  L 0 RDONLY,0x8000} 
20:23:19.338119 tx 50:     OK,  131072b data (fd data)
20:23:19.338235 rx 52: READ i5004682 {Fh 1 [131072 +131072)  L 0 RDONLY,0x8000} 
20:23:19.338242 tx 52:     OK,  131072b data (fd data)
20:23:19.338282 rx 54: READ i5004682 {Fh 1 [262144 +131072)  L 0 RDONLY,0x8000} 
20:23:19.338289 tx 54:     OK,  131072b data (fd data)
20:23:19.338342 rx 56: READ i5004682 {Fh 1 [393216 +131072)  L 0 RDONLY,0x8000} 
20:23:19.338353 tx 56:     OK,  131072b data (fd data)
20:23:19.338408 rx 58: READ i5004682 {Fh 1 [524288 +131072)  L 0 RDONLY,0x8000} 
20:23:19.338426 tx 58:     OK,  131072b data (fd data)
20:23:19.338454 rx 60: READ i5004682 {Fh 1 [655360 +131072)  L 0 RDONLY,0x8000} 
20:23:19.338460 tx 60:     OK,  131072b data (fd data)
20:23:19.338529 rx 62: READ i5004682 {Fh 1 [786432 +131072)  L 0 RDONLY,0x8000} 
20:23:19.338534 tx 62:     OK,  131072b data (fd data)
20:23:19.338587 rx 64: READ i5004682 {Fh 1 [917504 +131072)  L 0 RDONLY,0x8000} 
20:23:19.338592 tx 64:     OK,  131072b data (fd data)

@rfjakob
Copy link
Contributor

rfjakob commented Jan 4, 2022

PS: As for the public API, I think we should do what libfuse does: calculate MaxPages from MaxWrite ( libfuse/libfuse@027d0d1 ) and don't expose MaxPages to the user at all

@rfjakob
Copy link
Contributor

rfjakob commented Jan 6, 2022

I went down the rabbit hole about the read sizes a little bit:

A normal (=not direct i/o) read ends up in filemap_read(), and, quoting from the function comment:

 * Copies data from the page cache.  If the data is not currently present,
 * uses the readahead and readpage address_space operations to fetch it.

So it will use readahead to get the data. Following some indirections, the size of readahead requests for FUSE filesystems is ultimately limited by the default readahead size:

#define VM_READAHEAD_PAGES (SZ_128K / PAGE_SIZE)

For normal disks, you can change the readahead size via /sys/block/*/queue/read_ahead_kb, but FUSE uses the dummy disk device noop_backing_dev_info, and I don't see how you would change anything there.

Now because O_DIRECT bypasses readahead, the limit does not apply, so 1 MiB reads work with O_DIRECT:

dd if=b/foo iflag=direct bs=1M of=/dev/null

14:17:30.773557 rx 34: READ i5004682 {Fh 1 [0 +1048576) LOCKOWNER L 15366234143055493023 DIRECT,0x8000} 
14:17:30.773997 tx 34:     OK,  1048576b data (fd data)

@rfjakob
Copy link
Contributor

rfjakob commented Jan 9, 2022

@ShuranZhang
Copy link

May I ask if there are any possibilities to reconsider or continue on this issue? Since Linux now supports 1Mib i/o for up to 256 pages, but for this library it seems there is no way to configure up to 1Mib i/o. I am implementing a FUSE system that each read request will need to do some pre-processing, so enlarging the max page per read could significantly shorten the latency adds up from each read request.

@rfjakob
Copy link
Contributor

rfjakob commented Jun 29, 2022

Oh, looks like this got stuck on me. Maybe I'll find time to pick this up again.

@ShuranZhang
Copy link

Thank you for the response and I appreciate your contributions to this library! It has helped me a lot and I would like to quickly follow up on this opened issue. Sorry for keeping asking and I would love to pick up to see if there is anything I can do if you have no bandwidth recently.

@rfjakob
Copy link
Contributor

rfjakob commented Jul 31, 2022

Yeah you can update the patch to address Han-Wen's comments at https://review.gerrithub.io/c/hanwen/go-fuse/+/530724

@andrewchambers
Copy link
Contributor

Really interested in this change - It seems very close to the finish line.

@andrewchambers
Copy link
Contributor

andrewchambers commented Jan 21, 2023

It looks like the patch has been updated and as far as I can tell all the issues are resolved - though the gerrit interface is somewhat confusing to me.

@rfjakob
Copy link
Contributor

rfjakob commented Jan 21, 2023

No this is still unresolved: https://review.gerrithub.io/c/hanwen/go-fuse/+/530724/comments/15afc10b_ce660eff

Note that this went on the backburner for me because it did not significantly improve performance for gocryptfs.

@andrewchambers
Copy link
Contributor

@rfjakob doesn't this check test resolve the problem?

if srv.KernelSettings().Flags&fuse.CAP_MAX_PAGES == 0 {

afaik macos wouldn't set this flag and the tests will be skipped.

@rfjakob
Copy link
Contributor

rfjakob commented Jan 21, 2023

Yes.

@andrewchambers
Copy link
Contributor

@rfjakob if that is the case, what is the remaining work? or should @hanwen be able to approve/merge this now?

@rfjakob
Copy link
Contributor

rfjakob commented Jan 21, 2023

What do you mean? The check is not in the test...?

@rfjakob
Copy link
Contributor

rfjakob commented Jan 21, 2023

Oh I'm looking at an old version of the patchset.

@rfjakob
Copy link
Contributor

rfjakob commented Jan 21, 2023

Let me test this on an old kernel and I will ping Han-Wen if things pass.

hanwen pushed a commit that referenced this pull request Jan 29, 2023
Kernel 4.20 allows writes & reads up to 1 MiB (before: 128 kiB)
via CAP_MAX_PAGES & MaxPages.

Instead of exposing MaxPages in the API, we follow what libfuse
does, and calculate MaxPages from MaxWrite (rounding up).

Contrary to what libfuse does, we also set max_read to the same
value as MaxWrite. This prevents reads getting larger than writes
due to the rounding-up for MaxPages, which is unexpected. This
also changes the default behavoir of go-fuse, which was 64 kiB
writes, but 128 kiB for reads. Now it is 128 kiB for both.

The tests are implemented in the fs package because it's
easier there. They also test MaxReadAhead.

Tested on Linux 4.19.0 and Linux 6.1.7 via all.bash,
and on 6.1.7 also via the gocryptfs test suite.

Supersedes #347

Change-Id: I5a1d4ee91945155c367888da7a90814a24a9ee6e
@hanwen
Copy link
Owner

hanwen commented Apr 23, 2023

see 265a392

@hanwen hanwen closed this Apr 23, 2023
SandyXSD pushed a commit to juicedata/go-fuse that referenced this pull request Jun 26, 2023
Kernel 4.20 allows writes & reads up to 1 MiB (before: 128 kiB)
via CAP_MAX_PAGES & MaxPages.

Instead of exposing MaxPages in the API, we follow what libfuse
does, and calculate MaxPages from MaxWrite (rounding up).

Contrary to what libfuse does, we also set max_read to the same
value as MaxWrite. This prevents reads getting larger than writes
due to the rounding-up for MaxPages, which is unexpected. This
also changes the default behavoir of go-fuse, which was 64 kiB
writes, but 128 kiB for reads. Now it is 128 kiB for both.

The tests are implemented in the fs package because it's
easier there. They also test MaxReadAhead.

Tested on Linux 4.19.0 and Linux 6.1.7 via all.bash,
and on 6.1.7 also via the gocryptfs test suite.

Supersedes hanwen#347

Change-Id: I5a1d4ee91945155c367888da7a90814a24a9ee6e
CodingPoeta pushed a commit to juicedata/go-fuse that referenced this pull request Mar 29, 2024
Kernel 4.20 allows writes & reads up to 1 MiB (before: 128 kiB)
via CAP_MAX_PAGES & MaxPages.

Instead of exposing MaxPages in the API, we follow what libfuse
does, and calculate MaxPages from MaxWrite (rounding up).

Contrary to what libfuse does, we also set max_read to the same
value as MaxWrite. This prevents reads getting larger than writes
due to the rounding-up for MaxPages, which is unexpected. This
also changes the default behavoir of go-fuse, which was 64 kiB
writes, but 128 kiB for reads. Now it is 128 kiB for both.

The tests are implemented in the fs package because it's
easier there. They also test MaxReadAhead.

Tested on Linux 4.19.0 and Linux 6.1.7 via all.bash,
and on 6.1.7 also via the gocryptfs test suite.

Supersedes hanwen#347

Change-Id: I5a1d4ee91945155c367888da7a90814a24a9ee6e
CodingPoeta pushed a commit to juicedata/go-fuse that referenced this pull request May 8, 2024
Kernel 4.20 allows writes & reads up to 1 MiB (before: 128 kiB)
via CAP_MAX_PAGES & MaxPages.

Instead of exposing MaxPages in the API, we follow what libfuse
does, and calculate MaxPages from MaxWrite (rounding up).

Contrary to what libfuse does, we also set max_read to the same
value as MaxWrite. This prevents reads getting larger than writes
due to the rounding-up for MaxPages, which is unexpected. This
also changes the default behavoir of go-fuse, which was 64 kiB
writes, but 128 kiB for reads. Now it is 128 kiB for both.

The tests are implemented in the fs package because it's
easier there. They also test MaxReadAhead.

Tested on Linux 4.19.0 and Linux 6.1.7 via all.bash,
and on 6.1.7 also via the gocryptfs test suite.

Supersedes hanwen#347

Change-Id: I5a1d4ee91945155c367888da7a90814a24a9ee6e
CodingPoeta pushed a commit to juicedata/go-fuse that referenced this pull request May 8, 2024
Kernel 4.20 allows writes & reads up to 1 MiB (before: 128 kiB)
via CAP_MAX_PAGES & MaxPages.

Instead of exposing MaxPages in the API, we follow what libfuse
does, and calculate MaxPages from MaxWrite (rounding up).

Contrary to what libfuse does, we also set max_read to the same
value as MaxWrite. This prevents reads getting larger than writes
due to the rounding-up for MaxPages, which is unexpected. This
also changes the default behavoir of go-fuse, which was 64 kiB
writes, but 128 kiB for reads. Now it is 128 kiB for both.

The tests are implemented in the fs package because it's
easier there. They also test MaxReadAhead.

Tested on Linux 4.19.0 and Linux 6.1.7 via all.bash,
and on 6.1.7 also via the gocryptfs test suite.

Supersedes hanwen#347

Change-Id: I5a1d4ee91945155c367888da7a90814a24a9ee6e
CodingPoeta pushed a commit to juicedata/go-fuse that referenced this pull request May 8, 2024
Kernel 4.20 allows writes & reads up to 1 MiB (before: 128 kiB)
via CAP_MAX_PAGES & MaxPages.

Instead of exposing MaxPages in the API, we follow what libfuse
does, and calculate MaxPages from MaxWrite (rounding up).

Contrary to what libfuse does, we also set max_read to the same
value as MaxWrite. This prevents reads getting larger than writes
due to the rounding-up for MaxPages, which is unexpected. This
also changes the default behavoir of go-fuse, which was 64 kiB
writes, but 128 kiB for reads. Now it is 128 kiB for both.

The tests are implemented in the fs package because it's
easier there. They also test MaxReadAhead.

Tested on Linux 4.19.0 and Linux 6.1.7 via all.bash,
and on 6.1.7 also via the gocryptfs test suite.

Supersedes hanwen#347

Change-Id: I5a1d4ee91945155c367888da7a90814a24a9ee6e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants