Date: Wed, 13 Dec 2017 17:20:43 +0100
From: Boris Brezillon <boris.brezillon@free-electrons.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Wolfram Sang <wsa@the-dreams.de>, linux-i2c@vger.kernel.org,
        Jonathan Corbet <corbet@lwn.net>, linux-doc@vger.kernel.org,
        Arnd Bergmann <arnd@arndb.de>, Przemyslaw Sroka <psroka@cadence.com>,
        Arkadiusz Golec <agolec@cadence.com>,
        Alan Douglas <adouglas@cadence.com>,
        Bartosz Folta <bfolta@cadence.com>, Damian Kos <dkos@cadence.com>,
        Alicja Jurasik-Urbaniak <alicja@cadence.com>,
        Jan Kotas <jank@cadence.com>, Cyprian Wronka <cwronka@cadence.com>,
        Alexandre Belloni <alexandre.belloni@free-electrons.com>,
        Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
        Nishanth Menon <nm@ti.com>, Rob Herring <robh+dt@kernel.org>,
        Pawel Moll <pawel.moll@arm.com>, Mark Rutland <mark.rutland@arm.com>,
        Ian Campbell <ijc+devicetree@hellion.org.uk>,
        Kumar Gala <galak@codeaurora.org>, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [RFC 2/5] i3c: Add core I3C infrastructure
Message-ID: <20171213172043.24e4d4bc@bbrezillon>
In-Reply-To: <20170802021327.GB23033@kroah.com>
References: <1501518290-5723-1-git-send-email-boris.brezillon@free-electrons.com>
        <1501518290-5723-3-git-send-email-boris.brezillon@free-electrons.com>
        <20170801014021.GA20004@kroah.com>
        <20170801124801.38bbc431@bbrezillon>
        <20170801175133.GA30520@kroah.com>
        <20170801233001.4b08834f@bbrezillon>
        <20170802021327.GB23033@kroah.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2532
Lines: 62

Hi Greg,

On Tue, 1 Aug 2017 19:13:27 -0700
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

> > > > Unless you see a good reason to not use a R/W lock, I'd like to keep it
> > > > this way because master IPs are likely to implement advanced queuing
> > > > mechanism (allows one to queue new transfers even if the master is
> > > > already busy processing other requests), and serializing things at the
> > > > framework level will just prevent us from using this kind of
> > > > optimization.    
> > > 
> > > Unless you can prove otherwise, using a rw lock is almost always worse
> > > than just a mutex.  
> > 
> > Is it still true when it's taken in non-exclusive mode most of the
> > time, and the time you spend in the critical section is non-negligible?
> > 
> > I won't pretend I know better than you do what is preferable, it's just
> > that the RW lock seemed appropriate to me for the situation I tried to
> > described here.  
> 
> Again, measure it.  If you can't measure it, then don't use it.  Use a
> simple lock instead.  Seriously, don't make it more complex until you
> really have to.  It sounds like you didn't measure it at all, which
> isn't good, please do so.
> 

I'm resurrecting this thread because I finally had the time to implement
message queuing in Cadence I3C master driver. So I did a test with 2
I3C devices on the bus, and their drivers sending as much SDR messages
as they can in 10s. Here are the results:

          |    mutex    |    rwsem    |
---------------------------------------
dev1      |    19087    |    29532    |
dev2      |    19341    |    29118    |
=======================================
total     |    38428    |    58650    |
msg/sec	  |    ~3843    |    ~5865    |


The results I'm obtaining here are not so surprising since all normal
transfers are taking the lock in read mode, so there's no contention.
I didn't measure the impact on performances when there's one
maintenance operation taking the lock in write mode and several normal
transfers waiting for this lock, but really, maintenance operations are
infrequent, and that's not where performance matters in our use case.

I also did the same test with only one device doing transfers on the
bus, and this time the mutex wins, but there's not a huge difference.

          |    mutex    |    rwsem    |
---------------------------------------
total     |    67116    |    66561    |
msg/sec	  |    ~6712    |    ~6656    |

Let me know if you want more information on the test procedure.

Regards,

Boris