public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Dominique Martinet <asmadeus@codewreck.org>
To: Christian Schoenebeck <linux_oss@crudebyte.com>
Cc: Kent Overstreet <kent.overstreet@gmail.com>,
	linux-kernel@vger.kernel.org,
	v9fs-developer@lists.sourceforge.net,
	Eric Van Hensbergen <ericvh@gmail.com>,
	Latchesar Ionkov <lucho@ionkov.net>
Subject: Re: [PATCH 3/3] 9p: Add mempools for RPCs
Date: Sun, 10 Jul 2022 05:50:30 +0900	[thread overview]
Message-ID: <YsnqFqQodj9khp3Q@codewreck.org> (raw)
In-Reply-To: <2211309.MyIe47cYEz@silver>

Christian Schoenebeck wrote on Sat, Jul 09, 2022 at 08:08:41PM +0200:
> Mmm, I "think" that wouldn't be something new. There is no guarantee that 
> client would not get a late response delivery by server of a request that 
> client has already thrown away.

No. Well, it shouldn't -- responding to tflush should guarantee that the
associated request is thrown away by the server

https://9fans.github.io/plan9port/man/man9/flush.html

Order is not explicit, but I read this:
> If it recognizes oldtag as the tag of a pending transaction, it should
> abort any pending response and discard that tag.

late replies to the oldtag are no longer allowed once rflush has been
sent.

But I guess that also depends on the transport being sequential -- that
is the case for TCP but is it true for virtio as well? e.g. if a server
replies something and immediately replies rflush are we guaranteed
rflush is received second by the client?


There's also this bit:
> When the client sends a Tflush, it must wait to receive the
> corresponding Rflush before reusing oldtag for subsequent messages

if we free the request at this point we'd reuse the tag immediately,
which definitely lead to troubles.


> What happens on server side is: requests come in sequentially, and are started 
> to be processed exactly in that order. But then they are actually running in 
> parallel on worker threads, dispatched back and forth between threads several 
> times. And Tflush itself is really just another request. So there is no 
> guarantee that the response order corresponds to the order of requests 
> originally sent by client, and if client sent a Tflush, it might still get a 
> response to its causal, abolished "normal" request.

yes and processing flush ought to get a lock or something and look for
oldtag.
Looking at qemu code it does it right: processing flush find the old
request and marks it as cancelled, then it waits for the request to
finish (and possibly get discarded) during which (pdu_complete) it'll
wake the flush up; so spurrious replies of a tag after flush should not
be possible.

--
Dominique

  reply	other threads:[~2022-07-09 20:50 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20220704010945.C230AC341C7@smtp.kernel.org>
2022-07-04  1:42 ` [PATCH 1/3] 9p: Drop kref usage Kent Overstreet
2022-07-04  1:42   ` [PATCH 2/3] 9p: Add client parameter to p9_req_put() Kent Overstreet
2022-07-04  1:42   ` [PATCH 3/3] 9p: Add mempools for RPCs Kent Overstreet
2022-07-04  2:22     ` Dominique Martinet
2022-07-04  3:05       ` Kent Overstreet
2022-07-04  3:38         ` Dominique Martinet
2022-07-04  3:52           ` Kent Overstreet
2022-07-04 11:12           ` Christian Schoenebeck
2022-07-04 13:06             ` Dominique Martinet
2022-07-04 13:56               ` Christian Schoenebeck
2022-07-09  7:43                 ` Dominique Martinet
2022-07-09 14:21                   ` Christian Schoenebeck
2022-07-09 14:42                     ` Dominique Martinet
2022-07-09 18:08                       ` Christian Schoenebeck
2022-07-09 20:50                         ` Dominique Martinet [this message]
2022-07-10 12:57                           ` Christian Schoenebeck
2022-07-10 13:19                             ` Dominique Martinet
2022-07-10 15:16                               ` Christian Schoenebeck
2022-07-13  4:17                                 ` [RFC PATCH] 9p: forbid use of mempool for TFLUSH Dominique Martinet
2022-07-13  6:39                                   ` Kent Overstreet
2022-07-13  7:12                                     ` Dominique Martinet
2022-07-13  7:40                                       ` Kent Overstreet
2022-07-13  8:18                                         ` Dominique Martinet
2022-07-14 19:16                                   ` Christian Schoenebeck
2022-07-14 22:31                                     ` Dominique Martinet
2022-07-15 10:23                                       ` Christian Schoenebeck
2022-07-04 13:06             ` [PATCH 3/3] 9p: Add mempools for RPCs Kent Overstreet
2022-07-04 13:39               ` Christian Schoenebeck
2022-07-04 14:19                 ` Kent Overstreet
2022-07-05  9:59                   ` Christian Schoenebeck

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=YsnqFqQodj9khp3Q@codewreck.org \
    --to=asmadeus@codewreck.org \
    --cc=ericvh@gmail.com \
    --cc=kent.overstreet@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux_oss@crudebyte.com \
    --cc=lucho@ionkov.net \
    --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