Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 1 Dec 2020 15:57:28 +0100
From:   Dominique Martinet <asmadeus@codewreck.org>
To:     Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= <toke@redhat.com>
Cc:     Eric Van Hensbergen <ericvh@gmail.com>,
        Latchesar Ionkov <lucho@ionkov.net>,
        v9fs-developer@lists.sourceforge.net, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] fs: 9p: add generic splice_read file operations
Message-ID: <20201201145728.GA11144@nautica>
References: <20201201135409.55510-1-toke@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20201201135409.55510-1-toke@redhat.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk

Toke Høiland-Jørgensen wrote on Tue, Dec 01, 2020:
> The v9fs file operations were missing the splice_read operations, which
> breaks sendfile() of files on such a filesystem. I discovered this while
> trying to load an eBPF program using iproute2 inside a 'virtme' environment
> which uses 9pfs for the virtual file system. iproute2 relies on sendfile()
> with an AF_ALG socket to hash files, which was erroring out in the virtual
> environment.
> 
> Since generic_file_splice_read() seems to just implement splice_read in
> terms of the read_iter operation, I simply added the generic implementation
> to the file operations, which fixed the error I was seeing. A quick grep
> indicates that this is what most other file systems do as well.

Good catch, might as well do that.
I'm surprised you didn't hit the same problem with splice_write?

I see iter_file_splice_write being used for it on many filesystems,
it's probably better to add both?


> The only caveat is that my test case was only hitting the
> v9fs_file_operations_dotl implementation. I added it to the other file
> operations structs as well because it seemed like the sensible thing to do
> given that they all implement read_iter, but those are only compile tested.

The logic is close enough that it should work, I'll run it through in
cached mode at least first though (just mount with cache=loose or
cache=fscache to hit v9fs_cached_file_operations_dotl yourself if you
want to)
non-dotl operations are harder to test, I don't have any server
compatible either so we'll have to trust it works close enough...

(note you can write comments such as this one after the three dashes
line before the diff chunk so maintainers can reply without having it in
commit message itself)

-- 
Dominique