[vortex] tx_full eliminated

Tue, 16 May 2000 21:47:13 +0200 (CEST)

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.
  Send mail to mime@docserver.cac.washington.edu for more info.

---830399112-1291786407-958505307=:10843
Content-Type: TEXT/PLAIN; CHARSET=US-ASCII
Content-ID: <Pine.LNX.4.10.10005162132151.10843@kenzo.iwr.uni-heidelberg.de>

Hi,

I finally got time to look into the problem with tx_full which was the one
responsible for the race that I uncovered some weeks ago. The ideea is to
remove tx_full completely and always use cur_tx and dirty_tx with their
actual values; this way the situation when cur_tx = dirty_tx and tx_full =
1 is avoided.

I included a patch against Andrew's 2.2.16-pre2 driver, but I will specify
all the necessary changes (for Donald's line of drivers).

- remove the declaration of tx_full from the beginning of the driver

- in vortex_tx_timeout:
	- remove tx_full from one printk
	- change:
if (vp->tx_full && (vp->cur_tx - vp->dirty_tx <= TX_RING_SIZE - 1)) {
		vp->tx_full = 0;
		clear_bit(0, (void*)&dev->tbusy);
		}
to:
if (vp->cur_tx - vp->dirty_tx <= TX_RING_SIZE - 1)
	clear_bit(0, (void*)&dev->tbusy);

- in boomerang_start_xmit, change:
if (vp->tx_full) {
	if (vortex_debug ....
to:
if (vp->cur_tx - vp->dirty_tx >= TX_RING_SIZE) {
	if (vortex_debug ...
which will prevent running the rest of the routine if the ring is full.

and change:
if (vp->cur_tx - vp->dirty_tx > TX_RING_SIZE - 1)
	vp->tx_full = 1;
else 	{ /* Clear previous interrupt enable. */
	prev_entry->status &= cpu_to_le32(~TxIntrUploaded);
	clear_bit(0, (void*)&dev->tbusy);
	}
to:
if (vp->cur_tx - vp->dirty_tx <= TX_RING_SIZE - 1)
	{ /* Clear previous interrupt enable. */
	prev_entry->status &= cpu_to_le32(~TxIntrUploaded);
	clear_bit(0, (void*)&dev->tbusy);
	}

which only adds interrupt mitigation (in some drivers this part is
#ifdef-ed) only if the buffer is not full (cur_tx-dirty_tx=TX_RING_SIZE) -
if it's full we need a TxIntrUploaded on the last packet in the ring.  
The condition can probably be also written as: if (vp->cur_tx -
vp->dirty_tx != TX_RING_SIZE) because we were sure that cur_tx-dirty_tx <
TX_RING_SIZE (from the first test) and we just increment by 1.

- in vortex_interrupt, on the DownComplete branch, change: 
if (vp->tx_full && (vp->cur_tx - dirty_tx <= TX_RING_SIZE - 1)) {
	vp->tx_full = 0;
	clear_bit(0, (void*)&dev->tbusy);
	mark_bh(NET_BH);
	}
to:
if (vp->cur_tx - dirty_tx <= TX_RING_SIZE - 1) {
	clear_bit(0, (void*)&dev->tbusy);
	mark_bh(NET_BH);
	}

There is a difference here that I don't quite understand: in the original
code, the branch is only executed if the ring was full and there are some
entries available which should be almost every time, except if 
boomerang_start_xmit add packets at the same rhythm as vortex_interrupt
eats them (in which case the ring is kept full) - in this case we don't
clear tbusy, but also don't mark_bh. Why ?
In the proposed code, mark_bh is always called.

In testing this patch, I also moved the DownUnstall & spin_unlock in
boomerang_start_xmit in their original (as in Donald's drivers) place -
before vp->cur_tx++. This would expose the code to the race that I
experienced, but it's not happenning anymore after more than 8 hours
of testing (it used to take about 2 minutes to get it). This should also
allow these changes to work with Donald's new drivers (which have no I/O
operations in boomerang_start_xmit for non Boomerang cards).

The only thing that I haven't tested is the vortex_tx_timeout code which I
cannot reach even if I set TX_RING_SIZE = 2 and TX_TIMEOUT = 10. The tests
were: ping -f with different packet sizes, ttcp in both TCP and UDP mode
and some parallel jobs running on top of LAM-MPI (which uses TCP).

[The patch also has the loop counter after DownStall increased to 3000.]

Sincerely,

Bogdan Costescu

IWR - Interdisziplinaeres Zentrum fuer Wissenschaftliches Rechnen
Universitaet Heidelberg, INF 368, D-69120 Heidelberg, GERMANY
Telephone: +49 6221 54 8869, Telefax: +49 6221 54 8868
E-mail: Bogdan.Costescu@IWR.Uni-Heidelberg.De

---830399112-1291786407-958505307=:10843
Content-Type: TEXT/PLAIN; CHARSET=US-ASCII; NAME=p1
Content-Transfer-Encoding: BASE64
Content-ID: <Pine.LNX.4.10.10005162128270.10843@kenzo.iwr.uni-heidelberg.de>
Content-Description: 
Content-Disposition: ATTACHMENT; FILENAME=p1

LS0tIDIuMi4xNi1wcmUyLTNjNTl4LmMJTW9uIE1heSAxNSAxNjo1Nzo0MiAy
MDAwDQorKysgZnVsbC5jCVR1ZSBNYXkgMTYgMjE6MjA6MzkgMjAwMA0KQEAg
LTQ5MSw4ICs0OTEsNyBAQA0KIAkJZnVsbF9kdXBsZXg6MSwgZm9yY2VfZmQ6
MSwgYXV0b3NlbGVjdDoxLA0KIAkJYnVzX21hc3RlcjoxLAkJCQkvKiBWb3J0
ZXggY2FuIG9ubHkgZG8gYSBmcmFnbWVudCBidXMtbS4gKi8NCiAJCWZ1bGxf
YnVzX21hc3Rlcl90eDoxLCBmdWxsX2J1c19tYXN0ZXJfcng6MiwgLyogQm9v
bWVyYW5nICAqLw0KLQkJaHdfY3N1bXM6MSwJCQkJLyogSGFzIGhhcmR3YXJl
IGNoZWNrc3Vtcy4gKi8NCi0JCXR4X2Z1bGw6MTsNCisJCWh3X2NzdW1zOjE7
CQkJCS8qIEhhcyBoYXJkd2FyZSBjaGVja3N1bXMuICovDQogCXUxNiBzdGF0
dXNfZW5hYmxlOw0KIAl1MTYgaW50cl9lbmFibGU7DQogCXUxNiBhdmFpbGFi
bGVfbWVkaWE7CQkJCS8qIEZyb20gV24zX09wdGlvbnMuICovDQpAQCAtMTM2
Niw5ICsxMzY1LDkgQEANCiAjaWYgISBkZWZpbmVkKGZpbmFsX3ZlcnNpb24p
ICYmIExJTlVYX1ZFUlNJT05fQ09ERSA+PSAweDEwMzAwDQogCWlmICh2cC0+
ZnVsbF9idXNfbWFzdGVyX3R4KSB7DQogCQlpbnQgaTsNCi0JCXByaW50ayhL
RVJOX0RFQlVHICIgIEZsYWdzOyBidXMtbWFzdGVyICVkLCBmdWxsICVkOyBk
aXJ0eSAlZCAiDQorCQlwcmludGsoS0VSTl9ERUJVRyAiICBGbGFnczsgYnVz
LW1hc3RlciAlZCwgZGlydHkgJWQgIg0KIAkJCSAgICJjdXJyZW50ICVkLlxu
IiwNCi0JCQkgICB2cC0+ZnVsbF9idXNfbWFzdGVyX3R4LCB2cC0+dHhfZnVs
bCwgdnAtPmRpcnR5X3R4LCB2cC0+Y3VyX3R4KTsNCisJCQkgICB2cC0+ZnVs
bF9idXNfbWFzdGVyX3R4LCB2cC0+ZGlydHlfdHgsIHZwLT5jdXJfdHgpOw0K
IAkJcHJpbnRrKEtFUk5fREVCVUcgIiAgVHJhbnNtaXQgbGlzdCAlOC44eCB2
cy4gJXAuXG4iLA0KIAkJCSAgIGlubChpb2FkZHIgKyBEb3duTGlzdFB0ciks
DQogCQkJICAgJnZwLT50eF9yaW5nW3ZwLT5kaXJ0eV90eCAlIFRYX1JJTkdf
U0laRV0pOw0KQEAgLTEzODgsOCArMTM4Nyw3IEBADQogCQlpZiAodnAtPmN1
cl90eCAtIHZwLT5kaXJ0eV90eCA+IDAgICYmICBpbmwoaW9hZGRyICsgRG93
bkxpc3RQdHIpID09IDApDQogCQkJb3V0bCh2aXJ0X3RvX2J1cygmdnAtPnR4
X3JpbmdbdnAtPmRpcnR5X3R4ICUgVFhfUklOR19TSVpFXSksDQogCQkJCSBp
b2FkZHIgKyBEb3duTGlzdFB0cik7DQotCQlpZiAodnAtPnR4X2Z1bGwgJiYg
KHZwLT5jdXJfdHggLSB2cC0+ZGlydHlfdHggPD0gVFhfUklOR19TSVpFIC0g
MSkpIHsNCi0JCQl2cC0+dHhfZnVsbCA9IDA7DQorCQlpZiAodnAtPmN1cl90
eCAtIHZwLT5kaXJ0eV90eCA8PSBUWF9SSU5HX1NJWkUgLSAxKSB7DQogCQkJ
Y2xlYXJfYml0KDAsICh2b2lkKikmZGV2LT50YnVzeSk7DQogCQl9DQogCQlv
dXRiKFBLVF9CVUZfU1o+PjgsIGlvYWRkciArIFR4RnJlZVRocmVzaG9sZCk7
DQpAQCAtMTU4MSw3ICsxNTc5LDcgQEANCiAJCWlmICh2b3J0ZXhfZGVidWcg
PiAzKQ0KIAkJCXByaW50ayhLRVJOX0RFQlVHICIlczogVHJ5aW5nIHRvIHNl
bmQgYSBwYWNrZXQsIFR4IGluZGV4ICVkLlxuIiwNCiAJCQkJICAgZGV2LT5u
YW1lLCB2cC0+Y3VyX3R4KTsNCi0JCWlmICh2cC0+dHhfZnVsbCkgew0KKwkJ
aWYgKHZwLT5jdXJfdHggLSB2cC0+ZGlydHlfdHggPj0gVFhfUklOR19TSVpF
KSB7DQogCQkJaWYgKHZvcnRleF9kZWJ1ZyA+MCkNCiAJCQkJcHJpbnRrKEtF
Uk5fV0FSTklORyAiJXM6IFR4IFJpbmcgZnVsbCwgcmVmdXNpbmcgdG8gc2Vu
ZCBidWZmZXIuXG4iLA0KIAkJCQkJICAgZGV2LT5uYW1lKTsNCkBAIC0xNTk2
LDcgKzE1OTQsNyBAQA0KIAkJc3Bpbl9sb2NrX2lycXNhdmUoJnZwLT5sb2Nr
LCBmbGFncyk7DQogCQlvdXR3KERvd25TdGFsbCwgaW9hZGRyICsgRUwzX0NN
RCk7DQogCQkvKiBXYWl0IGZvciB0aGUgc3RhbGwgdG8gY29tcGxldGUuICov
DQotCQlmb3IgKGkgPSA2MDA7IGkgPj0gMCA7IGktLSkNCisJCWZvciAoaSA9
IDMwMDA7IGkgPj0gMCA7IGktLSkNCiAJCQlpZiAoIChpbncoaW9hZGRyICsg
RUwzX1NUQVRVUykgJiBDbWRJblByb2dyZXNzKSA9PSAwKQ0KIAkJCQlicmVh
azsNCiAJCXByZXZfZW50cnktPm5leHQgPSBjcHVfdG9fbGUzMih2aXJ0X3Rv
X2J1cygmdnAtPnR4X3JpbmdbZW50cnldKSk7DQpAQCAtMTYwNSwxNSArMTYw
MywxNSBAQA0KIAkJCXF1ZXVlZF9wYWNrZXQrKzsNCiAJCX0NCiANCisJCW91
dHcoRG93blVuc3RhbGwsIGlvYWRkciArIEVMM19DTUQpOw0KKwkJc3Bpbl91
bmxvY2tfaXJxcmVzdG9yZSgmdnAtPmxvY2ssIGZsYWdzKTsNCisNCiAJCXZw
LT5jdXJfdHgrKzsNCi0JCWlmICh2cC0+Y3VyX3R4IC0gdnAtPmRpcnR5X3R4
ID4gVFhfUklOR19TSVpFIC0gMSkNCi0JCQl2cC0+dHhfZnVsbCA9IDE7DQot
CQllbHNlIHsJCS8qIENsZWFyIHByZXZpb3VzIGludGVycnVwdCBlbmFibGUu
ICovDQorCQlpZiAodnAtPmN1cl90eCAtIHZwLT5kaXJ0eV90eCA8PSBUWF9S
SU5HX1NJWkUgLSAxKQ0KKwkJCXsJLyogQ2xlYXIgcHJldmlvdXMgaW50ZXJy
dXB0IGVuYWJsZS4gKi8NCiAJCQlwcmV2X2VudHJ5LT5zdGF0dXMgJj0gY3B1
X3RvX2xlMzIoflR4SW50clVwbG9hZGVkKTsNCiAJCQljbGVhcl9iaXQoMCwg
KHZvaWQqKSZkZXYtPnRidXN5KTsNCiAJCX0NCi0JCW91dHcoRG93blVuc3Rh
bGwsIGlvYWRkciArIEVMM19DTUQpOw0KLQkJc3Bpbl91bmxvY2tfaXJxcmVz
dG9yZSgmdnAtPmxvY2ssIGZsYWdzKTsNCiAJCWRldi0+dHJhbnNfc3RhcnQg
PSBqaWZmaWVzOw0KIAkJdnAtPnN0YXRzLnR4X2J5dGVzICs9IHNrYi0+bGVu
Ow0KIAkJcmV0dXJuIDA7DQpAQCAtMTY4MSw4ICsxNjc5LDcgQEANCiAJCQl9
DQogCQkJdnAtPmRpcnR5X3R4ID0gZGlydHlfdHg7DQogCQkJb3V0dyhBY2tJ
bnRyIHwgRG93bkNvbXBsZXRlLCBpb2FkZHIgKyBFTDNfQ01EKTsNCi0JCQlp
ZiAodnAtPnR4X2Z1bGwgJiYgKHZwLT5jdXJfdHggLSBkaXJ0eV90eCA8PSBU
WF9SSU5HX1NJWkUgLSAxKSkgew0KLQkJCQl2cC0+dHhfZnVsbD0gMDsNCisJ
CQlpZiAodnAtPmN1cl90eCAtIGRpcnR5X3R4IDw9IFRYX1JJTkdfU0laRSAt
IDEpIHsNCiAJCQkJY2xlYXJfYml0KDAsICh2b2lkKikmZGV2LT50YnVzeSk7
DQogCQkJCW1hcmtfYmgoTkVUX0JIKTsNCiAJCQl9DQo=
---830399112-1291786407-958505307=:10843--