Re: [uml-devel] Clearing kmalloc_ok during shutdown is broken - malloc will clear our data.

[Blaisorblade <blaisorblade@yahoo.it>]

On Wednesday 28 September 2005 22:12, Jeff Dike wrote:
> On Wed, Sep 28, 2005 at 01:46:15PM +0200, Blaisorblade wrote:
> > And I am wondering about whether the recent "eactivate_all_fds failed,
> > errno = 9"

> I found this one, although I didn't notice the missing 'd'.
Hmm, after saying that I saw that 'd' again.
In my experience with 2.6.13, it normally works, except when panicking (I 
triggered with sysrq t, and got the failure).

If it isn't a memory corruption, which I suspected, then we're ok.
> It turns out 
> that close_chan closes file descriptors, but never freed the associated
> IRQs.
So deactivate_all_fds() tries to close them again...
> > But still, Jeff, how can we expect that malloc won't stomp over all our
> > data which we preallocated with kmalloc and such?

> Because I leave a decent amount of room between the brk and the start of
> kmalloc-able memory (which is free_pages-d during boot).  It's a couple
> meg, I think.

> No guarantees, of course, but there isn't a lot of mallocing 
> happening.
Ok, this is enough, but could be documented. But we could record the end of 
malloc'able data, and check it, since we're using a wrapper anyway, and 
return NULL + my_printf()* a message if the space is exhausted.

* printf() can overflow kernel stack according to Al Viro, see 
not_configged_chan messages.


> > On the other side, could you explain why you don't like kmalloc in first
> > place? It surely works.

> I'm not sure - let me think about it.  But I fixed that for a reason - it
> was causing a crash somehow, I just don't remember the details.

> > The real solution for this warning is to replace um_kmalloc with
> > malloc(), and set, during shutdown, kmalloc_only_atomic - which would
> > switch
> > __wrap_malloc() from um_kmalloc to um_kmalloc_atomic.

> Yeah, that's userspace code, so it should probably just use malloc.

> > Or better yet, simply test in_atomic() and irqs_disabled() to choose
> > between the atomic and normal versions.

> I don't really like testing in_atomic() because you should generally know
> what context you're running in.
Beyond that, in_atomic() isn't set by spinlocks (at least not normally - it is 
set when preemption is enabled, not for us).

Userspace code calling malloc could miss this knowledge (as in the example 
above). Also userspace is supposed to use malloc, and with malloc there's no 
choice. And finally, surely libc isn't aware of anything such.
-- 
Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!".
Paolo Giarrusso, aka Blaisorblade (Skype ID "PaoloGiarrusso", ICQ 215621894)
http://www.user-mode-linux.org/~blaisorblade


	

	
		
___________________________________ 
Yahoo! Mail: gratis 1GB per i messaggi e allegati da 10MB 
http://mail.yahoo.it



-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel