Date: Thu, 17 Jan 2013 03:55:02 +0100 From: Luigi Rizzo <rizzo@iet.unipi.it> To: Barney Cordoba <barney_cordoba@yahoo.com> Cc: freebsd-net@freebsd.org Subject: Re: two problems in dev/e1000/if_lem.c::lem_handle_rxtx() Message-ID: <20130117025502.GA57613@onelab2.iet.unipi.it> In-Reply-To: <1358345941.38671.YahooMailClassic@web121604.mail.ne1.yahoo.com> References: <CA%2BhQ2%2BjEETBeN%2BHx=5aThr3pzcuK2L-%2BRd3cd1qDnYR67WSyxA@mail.gmail.com> <1358345941.38671.YahooMailClassic@web121604.mail.ne1.yahoo.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Jan 16, 2013 at 06:19:01AM -0800, Barney Cordoba wrote: > > > --- On Tue, 1/15/13, Luigi Rizzo <rizzo@iet.unipi.it> wrote: > > > From: Luigi Rizzo <rizzo@iet.unipi.it> > > Subject: two problems in dev/e1000/if_lem.c::lem_handle_rxtx() > > To: head@freebsd.org, "freebsd-net@freebsd.org" <freebsd-net@freebsd.org> > > Cc: "Jack Vogel" <jfvogel@gmail.com> > > Date: Tuesday, January 15, 2013, 8:23 PM > > Hi, > > i found a couple of problems in > > ? ? ? ? > > dev/e1000/if_lem.c::lem_handle_rxtx() , > > (compare with dev/e1000/if_em.c::em_handle_que() for better > > understanding): > > > > 1. in if_em.c::em_handle_que(), when em_rxeof() exceeds the > > ? rx_process_limit, the task is rescheduled so it can > > complete the work. > > ? Conversely, in if_lem.c::lem_handle_rxtx() the > > lem_rxeof() is > > ? only run once, and if there are more pending packets > > the only > > ? chance to drain them is to receive (many) more > > interrupts. > > > > ? This is a relatively serious problem, because the > > receiver has > > ? a hard time recovering. > > > > ? I'd like to commit a fix to this same as it is done > > in e1000. > > > > 2. in if_em.c::em_handle_que(), interrupts are reenabled > > unconditionally, > > ???whereas lem_handle_rxtx() only enables > > them if IFF_DRV_RUNNING is set. > > > > ???I cannot really tell what is the correct > > way here, so I'd like > > ???to put a comment there unless there is a > > good suggestion on > > ???what to do. > > > > ???Accesses to the intr register are > > race-prone anyways > > ???(disabled in fastintr, enabled in the rxtx > > task without > > ???holding any lock, and generally accessed > > under EM_CORE_LOCK > > ???in other places), and presumably > > enabling/disabling the > > ???interrupts around activations of the taks > > is just an > > ???optimization (and on a VM, it is actually > > a pessimization > > ???due to the huge cost of VM exits). > > > > cheers > > luigi > > This is not really a big deal; this is how things works for a million > years before we had task queues. i agree that the second issue is not a big deal. The first one, on the contrary, is a real problem no matter how you set the 'work' parameter (unless you make it large enough to drain the entire queue in one call). cheers luigi
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130117025502.GA57613>