Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 10 Jun 2002 02:32:24 -0700
From:      Mike Makonnen <makonnen@pacbell.net>
To:        John Baldwin <jhb@FreeBSD.ORG>
Cc:        freebsd-current@FreeBSD.ORG, jrh@lab.it.uc3m.es, hiten@uk.FreeBSD.org
Subject:   Re: Fixing "could sleeep..." was (Re: ../../../vm/uma_core.c:132
Message-ID:  <200206100932.g5A9WPe1008406@kokeb.ambesa.net>
In-Reply-To: <XFMail.20020608105732.jhb@FreeBSD.org>
References:  <200206081214.g58CEYmq006939@kokeb.ambesa.net> <XFMail.20020608105732.jhb@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, 08 Jun 2002 10:57:32 -0400 (EDT)
John Baldwin <jhb@FreeBSD.ORG> wrote:

> > 
> > Is the solution to this to use M_NOWAIT and continue re-trying
untill it> > succeeds? Is there on-going smp work in locking down struct
proc that> > will eliminate this problem?
> 
> Well, the real solution probably involves changing where we dink with
> uidinfo structs so we bump the reference count on teh new one before
we> grab the proc lock, change over to the new one while holding the
proc lock,> then release the reference to the old one after we are done.
> 

Well... this is basically what happens

setuid - creates a new ucred
	  - locks p
	  - calls change_ruid()

    change_ruid - calls uifind()

         uifind - does MALLOC w/ M_WAITOK

After thinking about it for a while, this is the solution I came up
with:

Each new struct ucred will carry an array of pointers to struct uidinfo.
This will be an array of 3 elements (a spare for cr_ruidinfo,
cr_uidinfo, null). Obviously, it gets added after ->cr_endcopy. 

When crget() is called it calls a function whose job it is to create an
array of pointers to struct uidinfo and allocate the memory for them.

When uifind() is called it will be given an array of pointers to uidinfo
structs (the ones from ucred), in addition to the uid it is to lookup. 
If it already has a uidinfo for that uid, then it returns that to the
calling function. If it can't find the uid, then it unhooks (copies the
address, and deletes it   from the array) the last struct uidinfo from
the array, initializes it, inserts it into the hashtable and returns it
to the caller.

When crfree is called it calls a function that deallocates the spare
structs uidinfo.

This solution has the advantage that the only code that has to change is
the ucred and setuid/gid helper functions that already know about the
struct uidinfo functions. In fact only three functions not related to
uidinfo(9) need to be touched: proc0_init(), change_ruid(),
change_uid(). The disadvantage is the memory bloat and a small amount of
code complexity (but as I said, this is localized, and not very complex
either).

Do you like it? 
Should I go ahead and implement a patch? 
Anything I overlooked?


Cheers,
Mike Makonnen

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




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200206100932.g5A9WPe1008406>