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@xxxxxxxxxxxxx > Fixes: eb497943fa21 ("9p: Convert to using the netfs helper lib to do reads > and caching") Cc: stable@xxxxxxxxxxxxxxx > Cc: David Howells <dhowells@xxxxxxxxxx> > Reported-By: Christian Schoenebeck <linux_oss@xxxxxxxxxxxxx> > Reviewed-by: Christian Schoenebeck <linux_oss@xxxxxxxxxxxxx> > Tested-by: Christian Schoenebeck <linux_oss@xxxxxxxxxxxxx> > Signed-off-by: Dominique Martinet <asmadeus@xxxxxxxxxxxxx> > --- > 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@xxxxxxxxxxxxx/ [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
