Date: Thu, 14 Jan 2010 10:10:31 -0800 From: Pyun YongHyeon <pyunyh@gmail.com> To: Igor Sysoev <is@rambler-co.ru> Cc: freebsd-net@freebsd.org Subject: Re: hw.bge.forced_collapse Message-ID: <20100114181031.GX1228@michelle.cdnetworks.com> In-Reply-To: <20100114160333.GC16657@rambler-co.ru> References: <20091204075440.GH14822@rambler-co.ru> <20091204173243.GC16491@michelle.cdnetworks.com> <20091204191114.GB76992@rambler-co.ru> <20091204195140.GH16491@michelle.cdnetworks.com> <20091204201303.GD76992@rambler-co.ru> <20091204202213.GI16491@michelle.cdnetworks.com> <20100114160333.GC16657@rambler-co.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
--p8PhoBjPxaQXD0vg
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
On Thu, Jan 14, 2010 at 07:03:33PM +0300, Igor Sysoev wrote:
> On Fri, Dec 04, 2009 at 12:22:13PM -0800, Pyun YongHyeon wrote:
>
> > On Fri, Dec 04, 2009 at 11:13:03PM +0300, Igor Sysoev wrote:
> > > On Fri, Dec 04, 2009 at 11:51:40AM -0800, Pyun YongHyeon wrote:
> > >
> > > > On Fri, Dec 04, 2009 at 10:11:14PM +0300, Igor Sysoev wrote:
> > > > > On Fri, Dec 04, 2009 at 09:32:43AM -0800, Pyun YongHyeon wrote:
> > > > >
> > > > > > On Fri, Dec 04, 2009 at 10:54:40AM +0300, Igor Sysoev wrote:
> > > > > > > I saw commit introducing hw.bge.forced_collapse loader tunable.
> > > > > > > Just intresting, why it can not be a sysctl ?
> > > > > >
> > > > > > I didn't think the sysctl variable would be frequently changed
> > > > > > in runtime except debugging driver so I took simple path.
> > > > >
> > > > > I do not think it's worth to reboot server just to look how various
> > > > > values affect on bandwidth and CPU usage, expecially in production.
> > > > >
> > > > > As I understand the change is trivial:
> > > > >
> > > > > - CTLFLAG_RD
> > > > > + CTLFLAG_RW
> > > > >
> > > > > since bge_forced_collapse is used atomically.
> > > > >
> > > >
> > > > I have no problem changing it to RW but that case I may have to
> > > > create actual sysctl node(e.g. dev.bge.0.forced_collapse) instead
> > > > of hw.bge.forced_collapse which may affect all bge(4) controllers
> > > > on system. Attached patch may be what you want. You can change the
> > > > value at any time.
> > >
> > > Thank you for the patch. Can it be installed on 8-STABLE ?
> > >
> >
> > bge(4) in HEAD has many fixes which were not MFCed to stable/8 so
> > I'm not sure that patch could be applied cleanly. But I guess you
> > can manually patch it.
> > I'll wait a couple of days for wider testing/review and commit the
> > patch.
>
> Sorry for the late response. We've tested bge.forced_collapse in December
> on HEAD and found that values >1 froze connections with big data amount,
> for example, "top -Ss1" output. Connection with small data amount such as
> short ssh commands worked OK. Now I've tested modern 7.2-STABLE and found
> that forced_collapse >1 freezes it too.
>
Thanks for reporting! It seems I've incorrectly dropped mbuf chains
when collapsing fails. Would you try attached patch?
--p8PhoBjPxaQXD0vg
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="bge.collapse.patch5"
Index: sys/dev/bge/if_bge.c
===================================================================
--- sys/dev/bge/if_bge.c (revision 202268)
+++ sys/dev/bge/if_bge.c (working copy)
@@ -3940,11 +3940,8 @@
m = m_defrag(m, M_DONTWAIT);
else
m = m_collapse(m, M_DONTWAIT, sc->bge_forced_collapse);
- if (m == NULL) {
- m_freem(*m_head);
- *m_head = NULL;
- return (ENOBUFS);
- }
+ if (m == NULL)
+ m = *m_head;
*m_head = m;
}
--p8PhoBjPxaQXD0vg--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100114181031.GX1228>