From: Dominique Martinet <asmadeus@codewreck.org>
To: Christian Schoenebeck <linux_oss@crudebyte.com>
Cc: Eric Van Hensbergen <ericvh@gmail.com>,
Latchesar Ionkov <lucho@ionkov.net>,
David Howells <dhowells@redhat.com>,
linux-fsdevel@vger.kernel.org, stable@vger.kernel.org,
v9fs-developer@lists.sourceforge.net,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] 9p: fix EBADF errors in cached mode
Date: Thu, 16 Jun 2022 23:11:16 +0900 [thread overview]
Message-ID: <Yqs6BPVc3rNZ9byJ@codewreck.org> (raw)
In-Reply-To: <Yqs1Y8G/Emi/q+S2@codewreck.org>
Dominique Martinet wrote on Thu, Jun 16, 2022 at 10:51:31PM +0900:
> > Did your patch work there for you? I mean I have not applied the other pending
> > 9p patches, but they should not really make difference, right? I won't have
> > time today, but I will continue to look at it tomorrow. If you already had
> > some thoughts on this, that would be great of course.
>
> Yes, my version passes basic tests at least, and I could no longer
> reproduce the problem.
For what it's worth I've also tested a version of your patch:
-----
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;
-----
And this also seems to work alright.
I was about to ask why the original code did writes with the writeback
fid, but I'm noticing now the current code still does (through
v9fs_vfs_write_folio_locked()), so that part hasn't changed from the old
code, and init_request will only be getting reads? Which actually makes
sense now I'm thinking about it because I recall David saying he's
working on netfs writes now...
So that minimal version is probably what we want, give or take style
adjustments (only initializing inode/v9inode in the if case or not) -- I
sure hope compilers optimizes it away when not needed.
I'll let you test one or both versions and will fixup the commit message
again/credit you/resend if we go with this version, unless you want to
send it.
--
Dominique
next prev parent reply other threads:[~2022-06-16 14:11 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CAAZOf26g-L2nSV-Siw6mwWQv1nv6on8c0fWqB4bKmX73QAFzow@mail.gmail.com>
2022-03-26 11:46 ` [syzbot] WARNING in p9_client_destroy David Kahurani
2022-03-26 11:48 ` Christian Schoenebeck
2022-03-26 12:24 ` asmadeus
2022-03-26 12:36 ` Christian Schoenebeck
2022-03-26 13:35 ` 9p fscache Duplicate cookie detected (Was: [syzbot] WARNING in p9_client_destroy) asmadeus
2022-03-30 12:21 ` 9p fs-cache tests/benchmark (was: 9p fscache Duplicate cookie detected) Christian Schoenebeck
2022-03-30 21:47 ` asmadeus
2022-04-01 14:19 ` Christian Schoenebeck
2022-04-01 23:11 ` asmadeus
2022-04-02 12:43 ` Christian Schoenebeck
2022-04-11 8:10 ` David Howells
2022-04-09 11:16 ` Christian Schoenebeck
2022-04-10 16:18 ` Christian Schoenebeck
2022-04-10 22:54 ` asmadeus
2022-04-11 13:41 ` Christian Schoenebeck
2022-04-12 22:38 ` asmadeus
2022-04-14 12:44 ` Christian Schoenebeck
2022-04-17 12:56 ` asmadeus
2022-04-17 13:52 ` Christian Schoenebeck
2022-04-17 21:22 ` asmadeus
2022-04-17 22:17 ` 9p EBADF with cache enabled (Was: 9p fs-cache tests/benchmark (was: 9p fscache Duplicate cookie detected)) asmadeus
2022-04-21 10:36 ` David Howells
2022-04-21 11:36 ` Christian Schoenebeck
2022-04-22 13:13 ` asmadeus
2022-04-25 14:10 ` David Howells
2022-04-26 15:38 ` Christian Schoenebeck
2022-05-03 10:21 ` asmadeus
2022-05-04 18:33 ` Christian Schoenebeck
2022-05-04 21:48 ` asmadeus
2022-05-06 19:14 ` Christian Schoenebeck
2022-06-03 16:46 ` Christian Schoenebeck
2022-06-12 10:02 ` asmadeus
2022-06-14 3:38 ` [PATCH] 9p: fix EBADF errors in cached mode Dominique Martinet
2022-06-14 3:41 ` Dominique Martinet
2022-06-14 12:10 ` Christian Schoenebeck
2022-06-14 12:45 ` Dominique Martinet
2022-06-14 14:11 ` Christian Schoenebeck
2022-06-16 13:35 ` Christian Schoenebeck
2022-06-16 13:51 ` Dominique Martinet
2022-06-16 14:11 ` Dominique Martinet [this message]
2022-06-16 20:14 ` Christian Schoenebeck
2022-06-16 20:53 ` Dominique Martinet
2022-06-16 21:10 ` [PATCH v3] " Dominique Martinet
2022-06-20 12:47 ` Christian Schoenebeck
2022-06-20 20:34 ` Dominique Martinet
2022-06-21 12:13 ` Christian Schoenebeck
2022-06-16 13:52 ` [PATCH v2] " Dominique Martinet
2022-04-11 7:59 ` 9p fs-cache tests/benchmark (was: 9p fscache Duplicate cookie detected) David Howells
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Yqs6BPVc3rNZ9byJ@codewreck.org \
--to=asmadeus@codewreck.org \
--cc=dhowells@redhat.com \
--cc=ericvh@gmail.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux_oss@crudebyte.com \
--cc=lucho@ionkov.net \
--cc=stable@vger.kernel.org \
--cc=v9fs-developer@lists.sourceforge.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).