qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Christian Schoenebeck <qemu_oss@crudebyte.com>
To: qemu-devel@nongnu.org
Cc: Greg Kurz <groug@kaod.org>
Subject: Re: [PATCH v6 4/5] 9pfs: T_readdir latency optimization
Date: Fri, 03 Jul 2020 20:27:20 +0200	[thread overview]
Message-ID: <2242974.z7sRGHuNfS@silver> (raw)
In-Reply-To: <20200703180821.0416ebe8@bahia.lan>

On Freitag, 3. Juli 2020 18:08:21 CEST Greg Kurz wrote:
> On Fri, 03 Jul 2020 10:08:09 +0200
> 
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > On Donnerstag, 2. Juli 2020 19:23:35 CEST Christian Schoenebeck wrote:
> > > > > Back to the actual topic: so what do we do about the mutex then?
> > > > > CoMutex
> > > > > for 9p2000.u and Mutex for 9p2000.L? I know you find that ugly, but
> > > > > it
> > > > > would just be a transitional measure.
> > > > 
> > > > That would ruin my day...
> > > > 
> > > > More seriously, the recent fix for a deadlock condition that was
> > > > present
> > > > for years proves that nobody seems to be using silly clients with
> > > > QEMU.
> > > > So I think we should just dump the lock and add a one-time warning in
> > > > the top level handlers when we detect a duplicate readdir request on
> > > > the same fid. This should be a very simple patch (I can take care of
> > > > it right away).
> > > 
> > > Looks like we have a consensus! Then I wait for your patch removing the
> > > lock, and for your feedback whether or not you see anything else in this
> > > patch set?
> > 
> > Please wait before you work on this patch. I really do think your patch
> > should be based on/after this optimization patch for one reason: if (and
> > even though it's a big if) somebody comes along with a silly client as
> > you named it, then your patch can simply be reverted, which would not be
> > possible if it's next.
> > 
> > So I would really suggest I change this patch here to go the ugly way with
> > 2 mutex types for readdir 9p2000.L vs 9p2000.L, and your patch would get
> > rid of that mess by removing the lock entirely, okay?
> 
> If someones ever comes along with a silly client, she will get warnings
> explaining that she might get silly results. If it turns out that we
> really need to support that for valid reasons, it will be okay to focus
> on the appropriate fix when the time comes, not now. So I don't say any
> real benefit to postponing the removal of the lock after your series, but
> I do at least see one benefit, it will reduce the level of noise.

Reasons:

1. Less work for me, as I would have more conflicts to resolve manually if
   your patch would come next.

2. The dual mutex change is just like 3 lines of code more -> trivial (and my 
   problem)

3. If somebody complains, I just have to revert your patch.

4. For you, it does neither mean more time nor more efforts, as you haven't 
   started to write the patch yet.

5. The actual outcome from your side is the same: you get what you want, the
   lock will be gone entirely after all anyway.

and most notably:

6. I don't see any advantage if your patch would come next.

Best regards,
Christian Schoenebeck




  reply	other threads:[~2020-07-03 18:28 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-19 15:10 [PATCH v6 0/5] 9pfs: readdir optimization Christian Schoenebeck
2020-04-19 15:00 ` [PATCH v6 1/5] tests/virtio-9p: added split readdir tests Christian Schoenebeck
2020-04-19 15:00 ` [PATCH v6 2/5] 9pfs: make v9fs_readdir_response_size() public Christian Schoenebeck
2020-04-19 15:02 ` [PATCH v6 3/5] 9pfs: add new function v9fs_co_readdir_many() Christian Schoenebeck
2020-04-30 11:42   ` Greg Kurz
2020-04-30 12:50     ` Christian Schoenebeck
2020-04-30 13:30       ` Greg Kurz
2020-05-01 14:04         ` Christian Schoenebeck
2020-05-04  9:18           ` Greg Kurz
2020-05-04 10:08             ` Christian Schoenebeck
2020-05-07 12:16             ` Christian Schoenebeck
2020-05-07 15:59               ` Greg Kurz
2020-04-19 15:06 ` [PATCH v6 4/5] 9pfs: T_readdir latency optimization Christian Schoenebeck
2020-06-03 17:16   ` Christian Schoenebeck
2020-06-29 16:39     ` Greg Kurz
2020-06-30 15:16       ` Christian Schoenebeck
2020-06-30 16:39         ` Greg Kurz
2020-06-30 18:00           ` Christian Schoenebeck
2020-07-01 10:09             ` Greg Kurz
2020-07-01 11:47               ` Christian Schoenebeck
2020-07-01 15:12                 ` Greg Kurz
2020-07-02 11:43                   ` Christian Schoenebeck
2020-07-02 15:35                     ` Greg Kurz
2020-07-02 17:23                       ` Christian Schoenebeck
2020-07-03  8:08                         ` Christian Schoenebeck
2020-07-03 16:08                           ` Greg Kurz
2020-07-03 18:27                             ` Christian Schoenebeck [this message]
2020-07-03 15:53                         ` Greg Kurz
2020-07-03 18:12                           ` Christian Schoenebeck
2020-04-19 15:07 ` [PATCH v6 5/5] 9pfs: clarify latency of v9fs_co_run_in_worker() 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=2242974.z7sRGHuNfS@silver \
    --to=qemu_oss@crudebyte.com \
    --cc=groug@kaod.org \
    --cc=qemu-devel@nongnu.org \
    /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).