Skip to content
This repository has been archived by the owner on Jan 6, 2023. It is now read-only.

Set msize for 9p mounting #25

Open
wants to merge 1 commit into
base: 0.7.0-clearcontainers
Choose a base branch
from

Conversation

MarioCarrilloA
Copy link

@MarioCarrilloA MarioCarrilloA commented May 5, 2017

By adding the msize value (524288 bytes)
provides an enhancement in I/O storage operations.
The following results are from I/O operations such as:
read, write, random read etc...,using different block
sizes, where the x results express how many times is better.

Read

bs random linear
64K 6x 7x
256K 13x 17x
512K 16x 13x
64MB 14x 14x
256MB 15x 15x
512MB 14x 15x

Write

bs random linear
64K 3x 3x
256K 3x 3x
512K 3x 3x
64MB 3x 3x
256MB 3x 3x
512MB 3x 3x

Signed-off-by: Mario Alfredo Carrillo Arevalo [email protected]

@grahamwhaley
Copy link

Great, nice to see :-)
We might want to mention that the other performance metrics (network, boot times etc.) are unaffected, and there is a minor (~2%) memory footprint increase.
Patch looks good to me, but I'll give it a build/run test before I ack it here.

@jcvenegas
Copy link

Nice to have performance back! and even better \o/ 2% memory degradation for the performance improved is something is really good. looks good +1

@grahamwhaley
Copy link

built and ran file.
lgtm

@MarioCarrilloA MarioCarrilloA force-pushed the 0.7.0-clearcontainers branch 2 times, most recently from 90bed68 to b2c3a36 Compare May 9, 2017 20:08
@jcvenegas
Copy link

jcvenegas commented May 11, 2017

+1

Approved with PullApprove

Copy link

@gorozco1 gorozco1 left a comment

Choose a reason for hiding this comment

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

Could you add comment in the code explaining why this magic number.

src/init.c Outdated
@@ -465,7 +465,7 @@ static int hyper_setup_shared(struct hyper_pod *pod)
}

if (mount(pod->share_tag, SHARED_DIR, "9p",
MS_MGC_VAL| MS_NODEV, "trans=virtio") < 0) {
MS_MGC_VAL| MS_NODEV, "trans=virtio,msize=524288") < 0) {

Choose a reason for hiding this comment

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

Could you add a comment in the code explaining why this magic number?

By adding the msize value (524288 bytes)
provides an enhancement in I/O storage operations.
The following results are from I/O operations such as:
read, write, random read etc...,using different block
sizes, where the x results express how many times is better.

Read

| bs    | random | linear  |
| ----- | ------ | ------- |
| 64K   | 6x     | 7x      |
| 256K  | 13x    | 17x     |
| 512K  | 16x    | 13x     |
| 64MB  | 14x    | 14x     |
| 256MB | 15x    | 15x     |
| 512MB | 14x    | 15x     |

Write

| bs    | random | linear  |
| ----- | ------ | ------- |
| 64K   | 3x     | 3x      |
| 256K  | 3x     | 3x      |
| 512K  | 3x     | 3x      |
| 64MB  | 3x     | 3x      |
| 256MB | 3x     | 3x      |
| 512MB | 3x     | 3x      |

Signed-off-by: Mario Alfredo Carrillo Arevalo <[email protected]>
@MarioCarrilloA MarioCarrilloA force-pushed the 0.7.0-clearcontainers branch from b2c3a36 to 9f8a166 Compare May 11, 2017 20:42
@MarioCarrilloA
Copy link
Author

@gorozco1 comments added :)

@devimc
Copy link

devimc commented May 11, 2017

I'd like to see the review of Chao P (or Anthony Xu). I remember he did something similar in CC 2.0 but with msize=131072

@chao-p
Copy link

chao-p commented May 12, 2017

I picked 128K(msize=131072) because I saw less performance increase when I use a bigger value. I suggest to compare the read/write speed in several configurations(512K vs 128K or even 64K...) to see which is better.

@MarioCarrilloA
Copy link
Author

Hi @chao-p , I have done that experiment using several msize configuration(64K, 128K, 256K, 512K etc...), with different block sizes and I/O operations (rand read, rand write, linear read and linear write) and I got the best results with 512K :), however I do not know which tool(s) did you use for that.

@chao-p
Copy link

chao-p commented May 13, 2017

@MarioCarrilloA Sounds good, then please use 512K. I used pts/aio-stress in phoronix-test-suite.

@mcastelino
Copy link

We need to see the relationship between this and the backing storage block size.

For example in the case of device mapper you see that the xfs logbsize=64k. Not sure how the two will interact.

/dev/mapper/docker-253:0-1841856-edd0b562208c3c47b81b7dea03f245cb9974d6f27d374a501a9c23730fd579aa on /var/lib/docker/devicemapper/mnt/edd0b562208c3c47b81b7dea03f245cb9974d6f27d374a501a9c23730fd579aa type xfs (rw,relatime,seclabel,nouuid,attr2,inode64,logbsize=64k,sunit=128,swidth=128,noquota)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants