<html><head></head><body>After the sprint I was on vacation. I'll try to get around to it soon. Christian <div class="gmail_quote">On August 12, 2019 4:45:19 PM GMT+02:00, Seth Forshee <seth.forshee@canonical.com> wrote:<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"> <pre class="k9mail">On Mon, Aug 12, 2019 at 02:31:38PM +0200, Stefan Bader wrote: <blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">On 26.07.19 14:59, Seth Forshee wrote: <blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;">On Fri, Jul 26, 2019 at 10:29:38AM +0200, Christian Brauner wrote: <blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">On Thu, Jul 25, 2019 at 03:26:01PM -0500, Seth Forshee wrote: <blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #fcaf3e; padding-left: 1ex;">On Mon, Jul 22, 2019 at 05:05:08PM +0200, Christian Brauner wrote: <blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #e9b96e; padding-left: 1ex;">On Fri, Jul 19, 2019 at 03:02:21PM -0500, Seth Forshee wrote: <blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ccc; padding-left: 1ex;">On Fri, Jul 19, 2019 at 05:50:46PM +0200, Christian Brauner wrote: <blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ccc; padding-left: 1ex;"> BugLink: <a href="https://bugs.launchpad.net/bugs/1837223">https://bugs.launchpad.net/bugs/1837223</a> This enabled O_DIRECT support for shiftfs if the underlay supports it. Currently shiftfs does not handle O_DIRECT if the underlay supports it. This is blocking dqlite - an essential part of LXD - from profiting from the performance benefits of O_DIRECT on suitable filesystems when used with async io such as aio or io_uring. Overlayfs cannot support this directly since the upper filesystem in overlay can be any filesystem. So if the upper filesystem does not support O_DIRECT but the lower filesystem does you're out of luck. Shiftfs does not suffer from the same problem since there is not concept of an upper filesystem in the same way that overlayfs has it. Essentially, shiftfs is a transparent shim relaying everything to the underlay while overlayfs' upper layer is not (completely). </blockquote> I get concerned any time shiftfs just copies up some non-trivial data from the lower filesystem, that shiftfs is going to get bypassed and some important metadata will not get propoerly updated in shiftfs. The question that immediately comes to mind in this case is whether i_size in the shiftfs indoe gets updated following an O_DIRECT write, and I suspect tha there are other similar cases to worry about. Have you </blockquote> I'm not following, what are "other similar cases to worry about" and are those already exposed? </blockquote> I just mean anything else which might change in the underlay as a result of I/O, e.g. timestamps. <blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #e9b96e; padding-left: 1ex;">Re: i_size for O_DIRECT I have not seen prior art for filesystems that use iomap for O_DIRECT that would fiddle with i_size so I didn't want to get into that game. That is to say, shiftfs only supports O_DIRECT if the underlying filesystem is iomap based. This was the same approach that overlayfs wanted to use. </blockquote> Are you saying that an O_DIRECT write that extends a file does not result in i_size being updated? That doesn't sound right ... </blockquote> For example, when overlayfs intended to introduce support for this (which they didn't because of their version of upper and lower) they did not mess with i_size. That's one of the reasons I didn't. Do you prefer to copy up attributes to make sure we're not missing anything? </blockquote> I think that something like this: fd = open(path, O_RDWR|O_DIRECT); lseek(fd, 0, SEEK_END); write(fd, data, size); fstat(fd, &stat); printf("%ld\n", (long)stat.st_size); Should print out the correct size (and similarly correct data for other attributes). Maybe it will with this patch, I'm just not certain that it will, so really I'm just asking whether you've tested this sort of thing. If it doesn't work, then it seems like we probably do need to copy up the attributes. Seth </blockquote>With the long discussion seemingly stopping unresolved I am not sure this patchset is ready for SRU or not. </blockquote> I think we are still waiting for Christian to get back with some test results here, so these patches should wait. </pre></blockquote></div> -- Sent from my Android device with K-9 Mail. Please excuse my brevity.</body></html>