hi, in an attempt to simplify the locking in netmap, i would like to code the poll handler along these lines netmap_poll() { ... /* PART1 */ revents = 0; /* see if any of the rings has data */ for (i = first_ring; i < last_ring; i++) { na->nm_txsync(ifp, i, 1); /* check for space */ if (kring->ring->avail > 0) revents |= POLLOUT; } if (!(revents & POLLOUT)) { selrecord(td, main_wq); /* apparently not, want wakeup */ } /* PART2 */ /* but then retry in case someone new data comes in */ for (i = first_ring; i < last_ring; i++) { na->nm_txsync(ifp, i, 1); /* check for space */ if (kring->ring->avail > 0) revents |= POLLOUT; } ... } The tx interrupt service routine would only do a *_tx_interrupt() { ... selwakeuppri(main_wq, PI_NET); ... } The existing code does not have the PART2 block, and wraps both "PART1" and the selwakeuppri() in a lock/unlock pair, so there is no race between the txsync()/test/selrecord() and selwakeup(). But the nesting of locks (*_txsync() has its own lock) complicates the code a lot, especially when attaching NICs and VALE switches, not to mention that PART1 can be fairly time consuming so I don't want other threads to spin waiting for it to finish. Hence i am willing to pay the cost of a retry phase and remove the "external" lock. My concern is whether i should do something to "undo" the selrecord() in case i find that some queue is available in the retry phase. Probably not (since the same problem would occur if another file descriptor becomes ready), but just wanted to be sure. cheers luigiReceived on Fri Jul 26 2013 - 13:03:48 UTC
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:39 UTC