Project

General

Profile

Bug #4013

backout 6910752/6968206: needs more work

Added by Robert Mustacchi over 6 years ago. Updated over 6 years ago.

Status:
Resolved
Priority:
Urgent
Category:
kernel
Start date:
2013-08-08
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:

Description

It's totally broken. Per Keith's RTI:

This is a straight reversion (as applied via patch -R) of the original
change [0] for 6910752 and 6968206; it contains no new or modified
changes.  Please consider this backout for expedited approval and
immediate application.

The original wad has caused or is suspected to have caused innumerable
other bug reports.  See for example 3855, 3195, 2430, 1069, 1032, and
Joyent's OS-2133 [1] (probably 1344) and OS-2415, probably the same as
several of the others.  There has been discussion of these and similar
bugs for years; see for example [2] and [3] as well as previous attempts
[4] at a backout.  The wad has been backed out by Nexenta and Joyent and
most likely every other illumos vendor who maintains a private
repository.  The wad as written introduces known races, is badly
commented in very bad English, does not clearly define the semantics of
the new locking strategy, and apparently provides no meaningful
improvement in the problem it was intended to solve.  While any one of
these reasons would suffice, I have chosen the universal reason given
for all thoroughly hopeless wads for the past 20+ years: "needs more
work".  If any wad deserves such infamy, this one does.

Since the original change was intended to improve performance by
breaking up m_mutex, I ran a few simple performance tests before the
backout and after.  These tests were run using large thread counts on
very fast CPUs with a very fast 12-SSD ZFS backing store attached to
2308 controllers with IT-mode firmware.  Such circumstances should
maximise the impact of lock contention on delivered performance.  The
results show that performance is actually improved slightly after the
backout.  While the tests are not sufficient in scope or scale to prove
this, they are sufficient to show that while the original wad did indeed
reduce lock contention, it had no positive effect on delivered
performance.  The original changes either attacked the wrong root cause
or had impact only under much different circumstances (e.g., on SPARC,
or with different fabrics or controllers).

Tests were run with sysbench 0.4.12 on SmartOS, in a zone with maximum
I/O shares.  No throttling was observed at any point in the tests.
Results and lockstat data may be found at
https://us-east.manta.joyent.com/wesolows/public/mptsas/... :

Before backout:

withchina.128k.64.dsync.out, lockstat.out.withchina.128k.64.dsync
withchina.512.64.dsync.out, lockstat.out.withchina.512.64.dsync
withchina.87.8.fsync.out, lockstat.out.withchina.87.8.fsync

After backout:

nochina.128k.64.dsync.out, lockstat.out.nochina.128k.64.dsync
nochina.512.64.dsync.out, lockstat.out.nochina.512.64.dsync
nochina.87.8.fsync.out, lockstat.out.nochina.87.8.fsync

Let's kill this with fire, please.

[0] https://github.com/illumos/illumos-gate/commit/236cbc79a831053e1ce0340d0fab0cd452fbcd61
[1] https://github.com/joyent/illumos-joyent/commit/59a2f91c88b8ace7906e853b6359d07495d63096#usr/src/uts/common/io/scsi/adapters/mpt_sas/mptsas.c
[2] http://comments.gmane.org/gmane.os.openindiana.general/6612
[3] http://echelog.com/logs/browse/openindiana/1356735600
[4] http://www.listbox.com/member/archive/182179/2013/01/sort/thread_rev/page/4/entry/9:380/20121204154052:E2C518A2-3E52-11E2-BA9F-860D956AFB9B/

History

#1

Updated by Robert Mustacchi over 6 years ago

  • Status changed from Pending RTI to Resolved

Resolved in da4badc008f69df74f592b0831d92baa6dfcee76.

Also available in: Atom PDF