Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 23 Sep 2000 14:46:44 -0400 (EDT)
From:      Bosko Milekic <bmilekic@technokratis.com>
To:        Chuck Paterson <cp@bsdi.com>
Cc:        freebsd-net@FreeBSD.ORG, freebsd-arch@FreeBSD.ORG
Subject:   Re: mbuf system and SMPng
Message-ID:  <Pine.BSF.4.21.0009231425080.7679-100000@jehovah.technokratis.com>
In-Reply-To: <200009231808.MAA18066@berserker.bsdi.com>

next in thread | previous in thread | raw e-mail | index | archive | help

On Sat, 23 Sep 2000, Chuck Paterson wrote:

> Bosko,
> 	It looks like you can reduce the number of mutex operations
> and the number of locked operations if you call m_mballoc and m_mballoc_wait
> with the mmbfree mutex held.
> 
> 	for the interim you may have to drop the lock in m_mballoc
> before acquiring Giant and  calling kmem_malloc with the M_NOWAIT flag.
> Eventually you should just be able to call kmem_malloc with the
> M_NOWAIT flag and not release you lock. 

	Right, now that I look at it, MGET(HDR) can indeed be simplified a
  bunch by doing it this way.

> You will be able to get rid of some of the the atomic_add_long()s.
> 
> 	For m_mballoc_wait you can get rid of the immediate re-acquire
> of the lock on entry. You would need to make a version of MGET which
> doesn't acquire the lock for use inside these routines. If you really
> want only one version of the code you could do this my creating a
> lower level macro the MGET calls, with macro aguments that are non-existance
> or (mtx_enter(...) and mtx_exit(...)). 

	Indeed, the lock being released before the asleep() being called in
  m_mballoc_wait() is somewhat of a race. I was planning to get rid of the
  asleep/await combo anyway (noted in my journal). In fact, MGET() can be
  called even with the mutex held, it will just result in recursive
  grabbing of the mutex, but it's better to just re-arrange the macro with
  a lower-level one, as you say (I noticed BSD/OS does something like
  this).

> 	You can then replace the asleep and await with a
> msleep(&mmbfree.m_mtx). The way the code is now you could end up
> with the wakeup occurring before you even call asleep and then
> never waking up. Also calling asleep and await are more expensive
> than a single call to msleep()

	Well, if it happens before asleep, it's not so bad because it will
  be woken up on the next MFREE(), since m_mballoc_wid will be incremented.
  But I agree that it should be changed to even rid ourselves of this race.

> 	You also don't want to be calling MBWAKEUP on every MFREE.
> You should hold the state you need to decided to do this under the mmbfree
> mutex which you are already getting and releasing. You don't
> want to call wakeupone if you don't have to, it is yet another
> locked operation.

	Actually, MBWAKEUP() first makes sure to check m_mballoc_wid, and
  does so with the lock held, so it will not call wakeup_one() if nobody is
  sleeping. Note that MBWAKEUP() is always called with the necessary mutex
  held (whether it be the mclfree or mmbfree one).

> 	I put this together real quick so if it isn't clear I'll be
> glad to expand.
> 
> Chuck

	Thanks, Chuck! I will be reviewing this tonight and rolling up a new
  diff. In the meantime, let me know if there's anything else...

  Cheers,

  Bosko Milekic
  bmilekic@technokratis.com




To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.21.0009231425080.7679-100000>