Update 9p mount syscall so mmap is rw

[egernst]

By default the mmap command for 9p is read only

Reason: generic_file_readonly_mmap is used by 9pfs Kernel side client
code, in case there is no caching.

For the guest OS, we need to ensure:

1. CONFIG_9P_FSCACHE is enabled in the guest kernel
2. v9f2_file_mmap is used instead of generic_file_readonly_mmap

For 2, we want cache=fscache to enable writable mmaps. This patch
addresses this requirement.

Fixes #92.

Signed-off-by: Eric Ernst [email protected]

[jcvenegas]

depends on clearcontainers/linux#3

[sboeuf]

LGTM

[mcastelino]

LGTM

[mcastelino]

Does this need clearcontainers/linux#3

What will happen if we have an older kernel.

[egernst]

As I look at this closer, I noticed a couple things:

1. we could use cache=mmap or
2. cache=fscache

I actually saw the failure go away in either case (seemingly the mmap resolves as rw), regardless of the guest kernel's config update. I'm asking Arindam for clarity on this next.

I'm hoping to take a look @ the performance numbers for this PR -- @grahamwhaley - can you help point me in the right direction (or help with a run if you aren't tracking this repo w/ localci?)

[grahamwhaley]

@egernst
Reading @rarindam's info on #92, the statement there is that the mmap will be readonly with un-cached 9p mounts, and thus it would seem either mmap or fscache caching would enable the rw mmap.
The relevant code I think will be these fs ops tables in the kernel 9p:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/9p/vfs_file.c#n669

Thus, I'm not convinced the kernel config update is necessary (but I have not dug into it). I'll let @rarindam comment on that.

For performance, sure, we can run the fs metrics. I will note here also, this msize setting improvement in the same area never did land in the hyperstart repo, and I believe it is not in our local agent either - that is also stuck on getting 'metrics'. We should combine the efforts:
clearcontainers/hyperstart#25

[egernst]

From my measurements, mmap is slightly more performant relative to fscache and meets our RW needs. Will update the PR to use this cache mode instead.

[rarindam]

Hi Eric, Graham, Okay first things first: From the 9pfs mount options, 2000L and acl are not required. These were part of my commandline, and I just pasted them, which I see have created a lot of confusion. So you can remove them, they are not relevant. Only thing required to be passed for 9p is cache=mmap OR fscache. Second: It was from reading the code as Graham pointed out, that I discovered that by default, only READ only mmap is enabled in 9p. Hence when testpmd tries to acces a RW mmap area for rte config, it fails. Note: mmap call with RW will not fail. But trying to acces it through write, will fail. The RW mmap is enabled (as part of different function table altogether) when one uses the mmap option while mounting. Third: Its always good to have cache enabled to mmap or fscache, because no one knows which code will be using mmap in RW mode, which is actually a pretty common occurence. Finally, if this issue is not being seen in 3.0 then there has been some change which I am not aware of, certainly for the better. Arindam On Aug 29, 2017, at 7:24 AM, Eric Ernst <[email protected]<mailto:[email protected]>> wrote: From my measurements, mmap is slightly more performant relative to fscache and meets our RW needs. Will update the PR to use this cache mode instead. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#93 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AbyHQ1VtPvL06Oe8k9Fu4NB-6jPAtbh_ks5sdB8DgaJpZM4PE_7Q>.

[egernst]

Thanks @rarindam

The issue is observed in 3.0 -- this pull request is being made to address and fix it. with cache=mmap, my issue is resolved.