Re: [External] Re: [PATCH] virtiofsd: add -o allow_directio|no_directio option

[张佳辰]

On Fri, Aug 21, 2020 at 9:40 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

On 8/21/20 1:58 PM, Daniel P. Berrangé wrote:
> On Fri, Aug 21, 2020 at 11:41:26AM +0800, Jiachen Zhang wrote:
>> Due to the commit 65da4539803373ec4eec97ffc49ee90083e56efd, the O_DIRECT
>> open flag of guest applications will be discarded by virtiofsd. While
>> this behavior makes it consistent with the virtio-9p scheme when guest
>> applications using direct I/O, we no longer have any chance to bypass
>> the host page cache.
>>
>> Therefore, we add a flag 'allow_directio' to lo_data. If '-o no_directio'
>> option is added, or none of '-o no_directio' or '-o allow_directio' is
>> added, the 'allow_directio' will be set to 0, and virtiofsd discards
>> O_DIRECT as before. If '-o allow_directio' is added to the stariting
>> command-line, 'allow_directio' will be set to 1, so that the O_DIRECT
>> flags will be retained and host page cache can be bypassed.
>>
>> Signed-off-by: Jiachen Zhang <zhangjiachen.jaycee@bytedance.com>
>> ---
>>  tools/virtiofsd/helper.c         |  4 ++++
>>  tools/virtiofsd/passthrough_ll.c | 20 ++++++++++++++------
>>  2 files changed, 18 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c
>> index 3105b6c23a..534ff52c64 100644
>> --- a/tools/virtiofsd/helper.c
>> +++ b/tools/virtiofsd/helper.c
>> @@ -180,6 +180,10 @@ void fuse_cmdline_help(void)
>>             "                               (0 leaves rlimit unchanged)\n"
>>             "                               default: min(1000000, fs.file-max - 16384)\n"
>>             "                                        if the current rlimit is lower\n"
>> +           "    -o allow_directio|no_directio\n"
>> +           "                               retain/discard O_DIRECT flags passed down\n"
>> +           "                               to virtiofsd from guest applications.\n"
>> +           "                               default: no_directio\n"
>>             );
>
> The standard naming convention from existing options is to use
> $OPTNAME and no_$OPTNAME.
>
> IOW, don't use the "allow_" prefix. The options should be just
> "directio" and "no_directio"

As we have 'max_idle_threads' (and not maxidlethreads), can we
use 'direct_io' instead?

Thanks. I will split them in the next version.

Jiachen

>
> Regards,
> Daniel
>