[PATCH 1/2][SRU][Disco] UBUNTU: SAUCE: shiftfs: add O_DIRECT support

[Stefan Bader]

On 26.07.19 14:59, Seth Forshee wrote:
> On Fri, Jul 26, 2019 at 10:29:38AM +0200, Christian Brauner wrote:
>> On Thu, Jul 25, 2019 at 03:26:01PM -0500, Seth Forshee wrote:
>>> On Mon, Jul 22, 2019 at 05:05:08PM +0200, Christian Brauner wrote:
>>>> On Fri, Jul 19, 2019 at 03:02:21PM -0500, Seth Forshee wrote:
>>>>> On Fri, Jul 19, 2019 at 05:50:46PM +0200, Christian Brauner wrote:
>>>>>> BugLink: https://bugs.launchpad.net/bugs/1837223
>>>>>>
>>>>>> 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).
>>>>>
>>>>> 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
>>>>
>>>> I'm not following, what are "other similar cases to worry about" and are
>>>> those already exposed?
>>>
>>> I just mean anything else which might change in the underlay as a result
>>> of I/O, e.g. timestamps.
>>>
>>>> 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.
>>>
>>> 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 ...
>>
>> 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?
> 
> 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
> 
With the long discussion seemingly stopping unresolved I am not sure this
patchset is ready for SRU or not.

-Stefan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20190812/8f3d33da/attachment.sig>