Re: [PATCH v6 4/5] 9pfs: T_readdir latency optimization

[Christian Schoenebeck]

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