Re: [PULL 11/23] Revert "graph-lock: Disable locking for now"

2023-07-10 Thread Klaus Jensen
On Jul 10 14:40, Kevin Wolf wrote:
> Am 10.07.2023 um 14:22 hat Klaus Jensen geschrieben:
> > On Jun 28 16:15, Kevin Wolf wrote:
> > > Now that bdrv_graph_wrlock() temporarily drops the AioContext lock that
> > > its caller holds, it can poll without causing deadlocks. We can now
> > > re-enable graph locking.
> > > 
> > > This reverts commit ad128dff0bf4b6f971d05eb4335a627883a19c1d.
> > > 
> > 
> > I'm seeing a pretty major performance regression on iothread-enabled
> > virtio-blk (and on some on-going iothread hw/nvme work) with this
> > applied. Something like ~300k iops prior to this vs ~200k after on my
> > set up. On master, virtio-blk is currently faster without an iothread
> > (~215k) than with (~200k).
> > 
> > I bisected the change in iops to this revert.
> 
> Is CONFIG_DEBUG_GRAPH_LOCK enabled in your build? If so, this is
> expected to cost some performance. If not, we need to take a look at
> what else is causing the regression.
> 
> Kevin

Argh. Doh. Yes, was enabled and made QUITE the difference.

Sorry for the noise. Thanks!


signature.asc
Description: PGP signature


Re: [PULL 11/23] Revert "graph-lock: Disable locking for now"

2023-07-10 Thread Kevin Wolf
Am 10.07.2023 um 14:22 hat Klaus Jensen geschrieben:
> On Jun 28 16:15, Kevin Wolf wrote:
> > Now that bdrv_graph_wrlock() temporarily drops the AioContext lock that
> > its caller holds, it can poll without causing deadlocks. We can now
> > re-enable graph locking.
> > 
> > This reverts commit ad128dff0bf4b6f971d05eb4335a627883a19c1d.
> > 
> 
> I'm seeing a pretty major performance regression on iothread-enabled
> virtio-blk (and on some on-going iothread hw/nvme work) with this
> applied. Something like ~300k iops prior to this vs ~200k after on my
> set up. On master, virtio-blk is currently faster without an iothread
> (~215k) than with (~200k).
> 
> I bisected the change in iops to this revert.

Is CONFIG_DEBUG_GRAPH_LOCK enabled in your build? If so, this is
expected to cost some performance. If not, we need to take a look at
what else is causing the regression.

Kevin


signature.asc
Description: PGP signature


Re: [PULL 11/23] Revert "graph-lock: Disable locking for now"

2023-07-10 Thread Klaus Jensen
On Jun 28 16:15, Kevin Wolf wrote:
> Now that bdrv_graph_wrlock() temporarily drops the AioContext lock that
> its caller holds, it can poll without causing deadlocks. We can now
> re-enable graph locking.
> 
> This reverts commit ad128dff0bf4b6f971d05eb4335a627883a19c1d.
> 

I'm seeing a pretty major performance regression on iothread-enabled
virtio-blk (and on some on-going iothread hw/nvme work) with this
applied. Something like ~300k iops prior to this vs ~200k after on my
set up. On master, virtio-blk is currently faster without an iothread
(~215k) than with (~200k).

I bisected the change in iops to this revert.


signature.asc
Description: PGP signature