<div dir="ltr"><div>Hi <span style="font-size:14px">Klaus</span>, </div><div><br></div><div>Thank you for your reply.</div><div><br></div><div>I agree with that handling the concurrent changing of host and guest is very important.</div><div><br></div><div>As my approach to data consistency...</div><div><br></div><div>* Function to update the inode cache (e.g. sf_inode_revalidate in vboxsf, HgfsRevalidate in vmhgfs) also will clear a page cache.</div><div>  Since sf_inode_revalidate clear the inode cache at the right time,</div><div>  this implementation would clear the page cache by using the correct stat information.</div><div><br></div><div>* If any of file size and last modification time are different, the page cache will be cleared.</div><div>  I think it is enough to verifying that the host file is not changed.</div><div>  This approach is same as vmhgfs.</div><div><br></div><div>* Before using page cache in the guest side, sf_inode_revalidate is executed.</div><div>  A page cache will be used when performing read, write, and seek.</div><div>  So, I implemented to call the sf_inode_revalidate in file_operations read_iter, write_iter, splice_read and llseek before performing the generic_* function.</div><div><br></div><div><br></div><div>As a test on the concurrent changing of the file,</div><div>I have confirmed that the page cache in the guest side is cleared when an file is updated on the host side.</div><div><br></div><div>1. On the guest side, put the file data in the page cache. (cat file)</div><div>2. On the host side, edit the file. (echo "host edit" >> file)</div><div>3. On the guest side, After editing the file, make sure that the changes on the host side is saved correctly. (echo "guest edit" >> file && cat file)</div><div><br></div><div>Certainly, the test in more severe conditions is not carried out.</div><div>If there is a better way, we'll try the test again.</div><div><br></div><div>Thanks,</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">2015-08-06 23:51 GMT+09:00 Klaus Espenlaub <span dir="ltr"><<a href="mailto:klaus.espenlaub@oracle.com" target="_blank">klaus.espenlaub@oracle.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Tatsuya,<br>
<span class=""><br>
On 06.08.2015 16:20, Tatsuya Karino wrote:<br>
> Here is another patch for the improvement of vboxsf.<br>
><br>
> The key point is using page cache in the system call read/write.<br>
> I created this patch by reference to the other file system(e.g. ext4,<br>
> nfs, vmhgfs).<br>
<br>
</span>Now that gets a lot more interesting... I hope you didn't look too much<br>
at the normal 'block based' filesystems, as vboxsf is logically much<br>
closer to a network filesystem like nfs/cifs/vmhgfs. It has to deal with<br>
others making concurrent changes, in the vboxsf case the host can change<br>
directory and file contents (from what I understood accesses are less<br>
critical but there can be still delays due to writes being handled later<br>
by the page cache)..<br>
<br>
I'm not too worried about truly concurrent accesses (as handling them<br>
100% correct is very expensive, and the current cache-less<br>
implementation doesn't meet this very difficult requirement either).<br>
<br>
This optimization has a big potential for presenting stale data if any<br>
of the assumptions it makes aren't true.<br>
<span class=""><br>
> In this patch...<br>
> * To use page cache, generic_file_read_iter/generic_file_write_iter will<br>
> be called from read/write.<br>
> * the page cache will be cleared in sf_inode_revalidate if necessary.<br>
> * sf_inode_revalidate will be called in a cache sensitive function like<br>
> read_iter, write_iter.<br>
> * sf_inode_revalidate will not call unnecessary stat.<br>
<br>
</span>Can you say a bit more how you approached data consistency? Benchmark<br>
results are one thing, but correct operation is far more important.<br>
<br>
Klaus<br>
<div><div class="h5"><br>
> I tested this code on a kernel 3.19 Linux guest.<br>
> This patch is provided under the MIT license.<br>
><br>
><br>
> # before apply this patch<br>
> vagrant@debian-jessie:/vagrant$ for i in {0..2}; do time cat<br>
> large-file>/dev/null ; done<br>
><br>
> real  0m0.045s<br>
> user  0m0.000s<br>
> sys 0m0.020s<br>
><br>
> real  0m0.031s<br>
> user  0m0.000s<br>
> sys 0m0.016s<br>
><br>
> real  0m0.045s<br>
> user  0m0.000s<br>
> sys 0m0.020s<br>
><br>
><br>
> # after apply this patch<br>
> vagrant@debian-jessie:/vagrant$ for i in {0..2}; do time cat<br>
> large-file>/dev/null ; done<br>
><br>
> real  0m0.140s<br>
> user  0m0.000s<br>
> sys 0m0.072s<br>
><br>
> real  0m0.004s<br>
> user  0m0.000s<br>
> sys 0m0.000s<br>
><br>
> real  0m0.004s<br>
> user  0m0.000s<br>
> sys 0m0.000s<br>
><br>
> Thanks,<br>
><br>
> --<br>
> Tatsuya Karino<br>
<br>
</div></div>_______________________________________________<br>
vbox-dev mailing list<br>
<a href="mailto:vbox-dev@virtualbox.org">vbox-dev@virtualbox.org</a><br>
<a href="https://www.virtualbox.org/mailman/listinfo/vbox-dev" rel="noreferrer" target="_blank">https://www.virtualbox.org/mailman/listinfo/vbox-dev</a><br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr">Tatsuya Karino</div></div>
</div>