Date: Tue, 30 Jan 2001 14:48:09 -0500 From: "Bosko Milekic" <bmilekic@technokratis.com> To: "Bruce Evans" <bde@zeta.org.au>, "Boris Popov" <bp@FreeBSD.org> Cc: <cvs-committers@FreeBSD.org>, <cvs-all@FreeBSD.org> Subject: Re: cvs commit: src/sys/kern kern_malloc.c src/sys/sys malloc.h Message-ID: <B0000467313@www.marianopolis.edu>
next in thread | raw e-mail | index | archive | help
Bruce Evans wrote:
>
> On Tue, 30 Jan 2001, Boris Popov wrote:
>
> > On Mon, 29 Jan 2001, Bosko Milekic wrote:
> >
> > > In this case, don't you agree that using malloc() with M_WAITOK
> > > would be more appropriate? In the broken state that it is,
> > > kmem_malloc() will panic if it can't find the address space in
> > > kmem_map _anyway_, as long as you're calling with M_WAITOK. Adding
> > > M_PANIC is redundant in this light.
>
> M_PANIC is good for handling all the code that uses M_NOWAIT.
I disagree because if we are calling malloc() with M_NOWAIT, we
*should* be ready to fail. And if we do fail, then we should fail as
gracefully as possible, and that means not panic()ing the machine. If that
means dropping whatever we're doing, freeing everything we need to, and
stopping to provide service, then so be it. The possibility of an
unavailable service due to resource shortage should not be handled with a
panic(), it should be handled with more active means of prevention. In
other words, I agree that failure to provide an important service is "A Bad
Thing," but that is an entirely seperate issue that is not [correctly]
solvable with malloc() calling panic().
What this change does is encourage people to not write defensive code,
in my opinion. M_WAITOK is provided for callers who are willing to wait and
presently, the behavior is implemented in such a way that if the map is
exhausted and we're M_WAITOK, then kmem_malloc() will take care of the
panic() itself. This, too, should be fixed eventually, but again, that is a
seperate issue.
Introducing an additional flag to malloc() to handle a map starvation
for the M_NOWAIT case is a bad idea. Code that absolutely MUST call
malloc() with M_NOWAIT and also panic if it cannot get anything is rare, to
say the least, and in its [rare] case, the panic() should be explicitly
called from said code (and not from malloc()).
> > Dunno, the line 189 in kern_malloc.c (rev 1.80) tells that the
> > return value can be NULL even with M_WAITOK. If this is not true then
> > this change is obviously wrong.
Clearly, that comment is wrong. It may be wise for whoever writes a new
allocator or modifies malloc() to strategize a little differently in the
future, though. We're headed toward a threaded kernel and the more and more
we thread things, the less and less we'll want to be waiting forever for
resource allocations, unless we implement a clear "preventive measure
against Bad Guys." For example, if we have one finite set of [evil] threads
which consume 80% of some resource (especially in the case of wired-down
pages) and then proceed to hold on to it indefinetely, we will have the
remainder of the threads batteling for 20% of the leftovers. If the
aformentionned set of evil threads keeps gnawing away at the same resource
and calls the allocator with M_WAITOK, each of those evil guys will get a
place on the sleep queue _indefinetely_, and so will be woken up one by one
at some point in time, to consume more resources. So we'll keep sinking
towards 10% available, 5% available, etc... until we're at a point where we
have something which is similiar, in effect, to deadlock. As I said, the
real solution would be to actively prevent such problematic cases from
occuring, but as a final measure, we may eventually look into not always
waiting forever, even in the M_WAITOK case.
> This seems to result from confusion about M_WAITOK. There was a lot of
> confusion about this in 386BSD-0.0. I thought that it was understood
> now. It's behaviour of causing malloc() to wait and never return NULL
> is even documented in malloc.9 :-).
>
> Bruce
Regards,
Bosko.
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?B0000467313>