lkml.org 
[lkml]   [2022]   [Jun]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v3] 9p: fix EBADF errors in cached mode
Date
On Donnerstag, 16. Juni 2022 23:10:25 CEST Dominique Martinet wrote:
> cached operations sometimes need to do invalid operations (e.g. read
> on a write only file)
> Historic fscache had added a "writeback fid", a special handle opened
> RW as root, for this. The conversion to new fscache missed that bit.
>
> This commit reinstates a slightly lesser variant of the original code
> that uses the writeback fid for partial pages backfills if the regular
> user fid had been open as WRONLY, and thus would lack read permissions.
>
> Link:
> https://lkml.kernel.org/r/20220614033802.1606738-1-asmadeus@codewreck.org
> Fixes: eb497943fa21 ("9p: Convert to using the netfs helper lib to do reads
> and caching") Cc: stable@vger.kernel.org
> Cc: David Howells <dhowells@redhat.com>
> Reported-By: Christian Schoenebeck <linux_oss@crudebyte.com>
> Reviewed-by: Christian Schoenebeck <linux_oss@crudebyte.com>
> Tested-by: Christian Schoenebeck <linux_oss@crudebyte.com>
> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
> ---
> v3: use the least permissive version of the patch that only uses
> writeback fid when really required
>
> If no problem shows up by then I'll post this patch around Wed 23 (next
> week) with the other stable fixes.
>
> fs/9p/vfs_addr.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c
> index a8f512b44a85..d0833fa69faf 100644
> --- a/fs/9p/vfs_addr.c
> +++ b/fs/9p/vfs_addr.c
> @@ -58,8 +58,21 @@ static void v9fs_issue_read(struct netfs_io_subrequest
> *subreq) */
> static int v9fs_init_request(struct netfs_io_request *rreq, struct file
> *file) {
> + struct inode *inode = file_inode(file);
> + struct v9fs_inode *v9inode = V9FS_I(inode);
> struct p9_fid *fid = file->private_data;
>
> + BUG_ON(!fid);
> +
> + /* we might need to read from a fid that was opened write-only
> + * for read-modify-write of page cache, use the writeback fid
> + * for that */
> + if (rreq->origin == NETFS_READ_FOR_WRITE &&
> + (fid->mode & O_ACCMODE) == O_WRONLY) {
> + fid = v9inode->writeback_fid;
> + BUG_ON(!fid);
> + }
> +
> refcount_inc(&fid->count);
> rreq->netfs_priv = fid;
> return 0;

Some more tests this weekend; all looks fine. It appears that this also fixed
the performance degradation that I reported early in this thread. Again,
benchmarks compiling a bunch of sources:

Case Linux kernel version msize cache duration (average)

A) EBADF fix only [1] 512000 loose 31m 14s
B) EBADF fix only [1] 512000 mmap 44m 1s
C) EBADF fix + clunk fixes [2] 512000 loose 29m 32s
D) EBADF fix + clunk fixes [2] 512000 mmap 44m 0s
E) 5.10.84 512000 loose 35m 5s
F) 5.10.84 512000 mmap 65m 5s

[1] 5.19.0-rc2 + EBADF fix v3 patch (alone):
https://lore.kernel.org/lkml/20220616211025.1790171-1-asmadeus@codewreck.org/

[2] 5.19.0-rc2 + EBADF fix v3 patch + clunk fix patches, a.k.a. 9p-next:
https://github.com/martinetd/linux/commit/b0017602fdf6bd3f344dd49eaee8b6ffeed6dbac

Conclusion: all thumbs in my possession pointing upwards. :)

Thanks Dominique!

Best regards,
Christian Schoenebeck


\
 
 \ /
  Last update: 2022-06-20 14:48    [W:1.116 / U:0.772 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site