Re: cred stuff..

2002-02-11 Thread Bruce Evans

On Fri, 8 Feb 2002, Julian Elischer wrote:

> I'd like to commit the code to keep the ucred across userland,
> with the code to clear it to NULL kept under DEBUG ifdefs.
>
> i.e.
>
> >  in trap(), ast() and syscall()
> >
> > if (td->td_ucred != p->p_ucred) {
> > PROC_LOCK(p);
> > if (td->td_ucred) {
> > crfree(td->td_ucred);
> > td->td_ucred = NULL;
> > }
> > if (p->p_ucred != NULL) {
> > td->td_ucred = crhold(p->p_ucred);
> > }
> > PROC_UNLOCK(p);
> > }

Please fix the style bugs in this before committing:
- explicit NULL in only one null pointer checks
- excessive braces for one of the ifs.

> and in userret() and ast()
>
> >#ifdef DEBUG  /*your choice of variable here*/
> > PROC_LOCK(p);
> > if (td->td_ucred) {
> > crfree(td->td_ucred);
> > td->td_ucred = NULL;
> > }
> > PROC_UNLOCK(p);
> >#endif

I think this is better left where it is in the functions that aquire
the locks.  It can then be done unconditionally, and not in a loop.

The style of the null pointer check in this is bug for bug compatible
with the corresponding one above.

Bruce


To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



RE: cred stuff..

2002-02-11 Thread Julian Elischer

I'm a little worried about invariants because the behaviour when 
INVARIANTS is set wil be different to teh behaviour when it is off, which
is 'strange' to say the least. Normally the behaviour si the same but you
just check for invariant conditions.


On Fri, 8 Feb 2002, John Baldwin wrote:

> 
> On 08-Feb-02 Julian Elischer wrote:
> > 
> > I'd like to commit the code to keep the ucred across userland,
> > with the code to clear it to NULL kept under DEBUG ifdefs.
> 
> Use INVARIANTS for the ifdef macro name, but sure.
> 
> -- 
> 
> John Baldwin <[EMAIL PROTECTED]>  <><  http://www.FreeBSD.org/~jhb/
> "Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/
> 


To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



RE: cred stuff..

2002-02-11 Thread Julian Elischer

John, (peter? others?)

How is it that getting a ucred reference is guarded by PROC_LOCK(p)
but freeing it is guarded by mtx_lock(&Giant); 
?

Call me naive, but shouldn't they be guarded by the same thing?

Julian


On Fri, 8 Feb 2002, Julian Elischer wrote:

> I'm a little worried about invariants because the behaviour when 
> INVARIANTS is set wil be different to teh behaviour when it is off, which
> is 'strange' to say the least. Normally the behaviour si the same but you
> just check for invariant conditions.
> 
> 
> On Fri, 8 Feb 2002, John Baldwin wrote:
> 
> > 
> > On 08-Feb-02 Julian Elischer wrote:
> > > 
> > > I'd like to commit the code to keep the ucred across userland,
> > > with the code to clear it to NULL kept under DEBUG ifdefs.
> > 
> > Use INVARIANTS for the ifdef macro name, but sure.
> > 
> > -- 
> > 
> > John Baldwin <[EMAIL PROTECTED]>  <><  http://www.FreeBSD.org/~jhb/
> > "Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/
> > 
> 
> 
> To Unsubscribe: send mail to [EMAIL PROTECTED]
> with "unsubscribe freebsd-current" in the body of the message
> 


To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



cred stuff..

2002-02-11 Thread Julian Elischer


I'd like to commit the code to keep the ucred across userland,
with the code to clear it to NULL kept under DEBUG ifdefs.


i.e.

>  in trap(), ast() and syscall()
>
> if (td->td_ucred != p->p_ucred) {
> PROC_LOCK(p);
> if (td->td_ucred) {
> crfree(td->td_ucred);
> td->td_ucred = NULL;
> }
> if (p->p_ucred != NULL) {
> td->td_ucred = crhold(p->p_ucred);
> }
> PROC_UNLOCK(p);
> }

and in userret() and ast()

>#ifdef DEBUG  /*your choice of variable here*/
> PROC_LOCK(p);
> if (td->td_ucred) {
> crfree(td->td_ucred);
> td->td_ucred = NULL; 
> }
> PROC_UNLOCK(p);
>#endif








To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



RE: cred stuff..

2002-02-11 Thread John Baldwin


On 09-Feb-02 Julian Elischer wrote:
> I'm a little worried about invariants because the behaviour when 
> INVARIANTS is set wil be different to teh behaviour when it is off, which
> is 'strange' to say the least. Normally the behaviour si the same but you
> just check for invariant conditions.

In this case it is providing the support for the implicit KASSERT(td_ucred !=
NULL)) everyhwere that td_ucred is used. :)   If it makes you feel better, put
it under INVARIANT_SUPPORT.  It is the same type of test though, as it is a way
of asserting that we aren't using td_ucred when a thread isn't in the kernel.

> On Fri, 8 Feb 2002, John Baldwin wrote:
> 
>> 
>> On 08-Feb-02 Julian Elischer wrote:
>> > 
>> > I'd like to commit the code to keep the ucred across userland,
>> > with the code to clear it to NULL kept under DEBUG ifdefs.
>> 
>> Use INVARIANTS for the ifdef macro name, but sure.
>> 
>> -- 
>> 
>> John Baldwin <[EMAIL PROTECTED]>  <><  http://www.FreeBSD.org/~jhb/
>> "Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/
>> 
> 

-- 

John Baldwin <[EMAIL PROTECTED]>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



Re: cred stuff..

2002-02-09 Thread Julian Elischer



On Sun, 10 Feb 2002, Bruce Evans wrote:

> On Sat, 9 Feb 2002, Julian Elischer wrote:
> 
> > On Sun, 10 Feb 2002, Bruce Evans wrote:
> >
> > > On Sat, 9 Feb 2002, Julian Elischer wrote:
> > > > AST is not always called
> > > > and userret is always called, but unfortunatly sometimes multiple times
> > >
> > > userret() isn't always called either in my version :-).  When I'm
> > > finished, it will never be called (but I might rename ast() to userret()
> > > since it is essenttially the unusual case for the original userret()).
> >
> > Any chance you could break that bit of your patch out and make it an
> > independently committable bit? (and commit it) ?
> 
> It's not really ready.

The trouble is that it's a moving target so unless you bit ethe bullet
and commit something, you are never ready to commit.


> 
> > I'd like to see SOMETHING always called because I need somewhere to plug
> > in the code that the KSE kernel needs to run on KSE processes at the user
> > boundary.
> 
> Uh, running it on every crossing of the user boundary would be a
> pessimization.  How often will it do more than check and find that there
> is nothing to do?  Just put it in userret() for now, but leave the code
> that is already outside of userret still outside.

I'd like the OPTION to force an upcall on any traps and syscalls.
hardware interrupts are not so interresting but they are at the present
time still moments when signals can be delivered and I'd like to have
access to all those moments to 'alter' them for KSE purposes should it be
a KSE process.

> 
> > > > if someone were to clean up AST/userret
> > > > it would be easier, but I am not sure I understand all the issues..
> > > >
> > > > Particularly the interraction between ast() and userret() and the various
> > > > possible ASTs
> > >
> > > Logically, it belongs at the end of userret(),
> >
> > Logically WHAT does?
> 
> The above [context deleted] code (free the ucred on return to user mode).

ah.

> 
> > > but I would prefer it
> > > to be immediately after all calls to userret() like it almost is now
> > > so that I don't have to change it.  ast() is not special here, modulo
> > > optimizations -- it is just one caller of userret().  Think of it as
> > > just an optimization of one case of trap().
> >
> > but AST is called (potentially) from all users of doreti()
> > which you say includes fast interrupts.
> 
> It currently never has a ucred there.  It checks for this, and always
> allocates one, and always frees this ucred after it calls userret().
> Your version couldn't do things much differently (if ucreds are freed
> at all) without doing some very messy optimizations to keep the ucred
> until ast() is called but not keep it when ast() will not be called.

I am happy to keep the ucred all teh way into userspace and back
so I would never need any such optimisations. Just always keep it.




To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



Re: cred stuff..

2002-02-09 Thread Bruce Evans

On Sat, 9 Feb 2002, Julian Elischer wrote:

> On Sun, 10 Feb 2002, Bruce Evans wrote:
>
> > On Sat, 9 Feb 2002, Julian Elischer wrote:
> > > AST is not always called
> > > and userret is always called, but unfortunatly sometimes multiple times
> >
> > userret() isn't always called either in my version :-).  When I'm
> > finished, it will never be called (but I might rename ast() to userret()
> > since it is essenttially the unusual case for the original userret()).
>
> Any chance you could break that bit of your patch out and make it an
> independently committable bit? (and commit it) ?

It's not really ready.

> I'd like to see SOMETHING always called because I need somewhere to plug
> in the code that the KSE kernel needs to run on KSE processes at the user
> boundary.

Uh, running it on every crossing of the user boundary would be a
pessimization.  How often will it do more than check and find that there
is nothing to do?  Just put it in userret() for now, but leave the code
that is already outside of userret still outside.

> > > if someone were to clean up AST/userret
> > > it would be easier, but I am not sure I understand all the issues..
> > >
> > > Particularly the interraction between ast() and userret() and the various
> > > possible ASTs
> >
> > Logically, it belongs at the end of userret(),
>
> Logically WHAT does?

The above [context deleted] code (free the ucred on return to user mode).

> > but I would prefer it
> > to be immediately after all calls to userret() like it almost is now
> > so that I don't have to change it.  ast() is not special here, modulo
> > optimizations -- it is just one caller of userret().  Think of it as
> > just an optimization of one case of trap().
>
> but AST is called (potentially) from all users of doreti()
> which you say includes fast interrupts.

It currently never has a ucred there.  It checks for this, and always
allocates one, and always frees this ucred after it calls userret().
Your version couldn't do things much differently (if ucreds are freed
at all) without doing some very messy optimizations to keep the ucred
until ast() is called but not keep it when ast() will not be called.

> > I prefer explicit comparisions with NULL and 0 except for booleans.
>
> Actually so do I  though in this case I'm thinking of "if (x != NULL)"
> as "if X is valid"
>
> which comes out of my fingers as  "if (x)"  (validity is a boolean)
>
> Sometimes I've thought about something like:
>
> if (ISVALID(x))
>
> where ISVALID could be different on different architectures,
> (e.g. ia64 actually has support for item validity)
>
> but it's not worth it and doesn't gain anything except bloat.

It also gains obfuscation.

Bruce


To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



Re: cred stuff..

2002-02-09 Thread Julian Elischer



On Sun, 10 Feb 2002, Bruce Evans wrote:

> On Sat, 9 Feb 2002, Julian Elischer wrote:
> 
> >
> > AST is not always called
> > and userret is always called, but unfortunatly sometimes multiple times
> 
> userret() isn't always called either in my version :-).  When I'm
> finished, it will never be called (but I might rename ast() to userret()
> since it is essenttially the unusual case for the original userret()).

Any chance you could break that bit of your patch out and make it an
independently committable bit? (and commit it) ?

I'd like to see SOMETHING always called because I need somewhere to plug
in the code that the KSE kernel needs to run on KSE processes at the user
boundary. 


> 
> > if someone were to clean up AST/userret
> > it would be easier, but I am not sure I understand all the issues..
> >
> > Particularly the interraction between ast() and userret() and the various
> > possible ASTs
> 
> Logically, it belongs at the end of userret(),

Logically WHAT does?

> but I would prefer it
> to be immediately after all calls to userret() like it almost is now
> so that I don't have to change it.  ast() is not special here, modulo
> optimizations -- it is just one caller of userret().  Think of it as
> just an optimization of one case of trap().
> 

but AST is called (potentially) from all users of doreti()
which you say includes fast interrupts.



> 
> I prefer explicit comparisions with NULL and 0 except for booleans.

Actually so do I  though in this case I'm thinking of "if (x != NULL)"
as "if X is valid"

which comes out of my fingers as  "if (x)"  (validity is a boolean)

Sometimes I've thought about something like:

if (ISVALID(x))

where ISVALID could be different on different architectures,
(e.g. ia64 actually has support for item validity)

but it's not worth it and doesn't gain anything except bloat.

> 
> Bruce
> 
> 


To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



Re: cred stuff..

2002-02-09 Thread Bruce Evans

On Sat, 9 Feb 2002, Julian Elischer wrote:

> On Sun, 10 Feb 2002, Bruce Evans wrote:
>
> > On Fri, 8 Feb 2002, Julian Elischer wrote:
> ...
> > > and in userret() and ast()
> > >
> > > >#ifdef DEBUG  /*your choice of variable here*/
> > > > if (td->td_ucred != NULL) {
> > > > mtx_lock(&Giant);
> > > > crfree(td->td_ucred);
> > > > td->td_ucred = NULL;
> > > > mtx_unlock(&Giant);
> > > > }
> > > >#endif
> >
> > I think this is better left where it is in the functions that aquire
> > the locks.  It can then be done unconditionally, and not in a loop.
>
> AST is not always called
> and userret is always called, but unfortunatly sometimes multiple times

userret() isn't always called either in my version :-).  When I'm
finished, it will never be called (but I might rename ast() to userret()
since it is essenttially the unusual case for the original userret()).

> if someone were to clean up AST/userret
> it would be easier, but I am not sure I understand all the issues..
>
> Particularly the interraction between ast() and userret() and the various
> possible ASTs

Logically, it belongs at the end of userret(), but I would prefer it
to be immediately after all calls to userret() like it almost is now
so that I don't have to change it.  ast() is not special here, modulo
optimizations -- it is just one caller of userret().  Think of it as
just an optimization of once case of trap().

> > The style of the null pointer check in this is bug for bug compatible
> > with the corresponding one above.
>
> which way would you prefer?

I prefer explicit comparisions with NULL and 0 except for booleans.

Bruce


To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



Re: cred stuff..

2002-02-09 Thread Julian Elischer



On Sun, 10 Feb 2002, Bruce Evans wrote:

> On Fri, 8 Feb 2002, Julian Elischer wrote:
> 
> > I'd like to commit the code to keep the ucred across userland,
> > with the code to clear it to NULL kept under DEBUG ifdefs.
> >
> > i.e.
> >
> > >  in trap(), ast() and syscall()
> > >
> > > if (td->td_ucred != p->p_ucred) {
> > > if (td->td_ucred != NULL) {
> > > mtx_lock(&Giant);
> > > crfree(td->td_ucred);
> > > td->td_ucred = NULL;
> > > mtx_unlock(&Giant);
> > > }
> > > if (p->p_ucred != NULL) {
> > > PROC_LOCK(p);
> > > td->td_ucred = crhold(p->p_ucred);
> > > PROC_UNLOCK(p);
> > > }
> > > }
> 
> Please fix the style bugs in this before committing:
> - explicit NULL in only one null pointer checks
> - excessive braces for one of the ifs.

fixed

> 
> > and in userret() and ast()
> >
> > >#ifdef DEBUG  /*your choice of variable here*/
> > > if (td->td_ucred != NULL) {
> > > mtx_lock(&Giant);
> > > crfree(td->td_ucred);
> > > td->td_ucred = NULL;
> > > mtx_unlock(&Giant);
> > > }
> > >#endif
> 
> I think this is better left where it is in the functions that aquire
> the locks.  It can then be done unconditionally, and not in a loop.

AST is not always called
and userret is always called, but unfortunatly sometimes multiple times
if someone were to clean up AST/userret
it would be easier, but I am not sure I understand all the issues..

Particularly the interraction between ast() and userret() and the various
possible ASTs

> 
> The style of the null pointer check in this is bug for bug compatible
> with the corresponding one above.

which way would you prefer?

> 
> Bruce
> 
> 


To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



RE: cred stuff..

2002-02-09 Thread John Baldwin


On 09-Feb-02 Julian Elischer wrote:
> 
> 
> On Fri, 8 Feb 2002, Julian Elischer wrote:
> 
>> John, (peter? others?)
>> 
>> How is it that getting a ucred reference is guarded by PROC_LOCK(p)
>> but freeing it is guarded by mtx_lock(&Giant); 
>> ?
>> 
>> Call me naive, but shouldn't they be guarded by the same thing?
> 
> In fact, if we had good atomic reference counting primatives, 
> couldn't we just forget the locks entirely for this?

We had a long bikeshed about that a while back but people complained that
refcount_t wasn't generic enough, although it is well-suited to this specific
instance.  We can always come back and optimize the ucred mutex later, but
there are really more important things to be doing at this point. :)

It works for now, Giant is a much bigger problem that the ucred mutex.
 
>> Julian
>> 
>> 
>> On Fri, 8 Feb 2002, Julian Elischer wrote:
>> 
>> > I'm a little worried about invariants because the behaviour when 
>> > INVARIANTS is set wil be different to teh behaviour when it is off, which
>> > is 'strange' to say the least. Normally the behaviour si the same but you
>> > just check for invariant conditions.
>> > 
>> > 
>> > On Fri, 8 Feb 2002, John Baldwin wrote:
>> > 
>> > > 
>> > > On 08-Feb-02 Julian Elischer wrote:
>> > > > 
>> > > > I'd like to commit the code to keep the ucred across userland,
>> > > > with the code to clear it to NULL kept under DEBUG ifdefs.
>> > > 
>> > > Use INVARIANTS for the ifdef macro name, but sure.
>> > > 
>> > > -- 
>> > > 
>> > > John Baldwin <[EMAIL PROTECTED]>  <><  http://www.FreeBSD.org/~jhb/
>> > > "Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/
>> > > 
>> > 
>> > 
>> > To Unsubscribe: send mail to [EMAIL PROTECTED]
>> > with "unsubscribe freebsd-current" in the body of the message
>> > 
>> 
>> 
>> To Unsubscribe: send mail to [EMAIL PROTECTED]
>> with "unsubscribe freebsd-current" in the body of the message
>> 
> 

-- 

John Baldwin <[EMAIL PROTECTED]>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



RE: cred stuff..

2002-02-09 Thread John Baldwin


On 09-Feb-02 Julian Elischer wrote:
> John, (peter? others?)
> 
> How is it that getting a ucred reference is guarded by PROC_LOCK(p)
> but freeing it is guarded by mtx_lock(&Giant); 
> ?
> 
> Call me naive, but shouldn't they be guarded by the same thing?

Naive, maybe. :-P  The actual refcount for a ucred is protected by an internal
mutex.  The PROC_LOCK is to ensure that the value of p_ucred is up to date and
doesn't change so that the cred doesn't get free'd out from under us.  Giant is
needed for crfree() since it can call free().

> Julian
> 
> 
> On Fri, 8 Feb 2002, Julian Elischer wrote:
> 
>> I'm a little worried about invariants because the behaviour when 
>> INVARIANTS is set wil be different to teh behaviour when it is off, which
>> is 'strange' to say the least. Normally the behaviour si the same but you
>> just check for invariant conditions.
>> 
>> 
>> On Fri, 8 Feb 2002, John Baldwin wrote:
>> 
>> > 
>> > On 08-Feb-02 Julian Elischer wrote:
>> > > 
>> > > I'd like to commit the code to keep the ucred across userland,
>> > > with the code to clear it to NULL kept under DEBUG ifdefs.
>> > 
>> > Use INVARIANTS for the ifdef macro name, but sure.
>> > 
>> > -- 
>> > 
>> > John Baldwin <[EMAIL PROTECTED]>  <><  http://www.FreeBSD.org/~jhb/
>> > "Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/
>> > 
>> 
>> 
>> To Unsubscribe: send mail to [EMAIL PROTECTED]
>> with "unsubscribe freebsd-current" in the body of the message
>> 
> 

-- 

John Baldwin <[EMAIL PROTECTED]>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



RE: cred stuff..

2002-02-09 Thread John Baldwin


On 09-Feb-02 Julian Elischer wrote:
> I'm a little worried about invariants because the behaviour when 
> INVARIANTS is set wil be different to teh behaviour when it is off, which
> is 'strange' to say the least. Normally the behaviour si the same but you
> just check for invariant conditions.

In this case it is providing the support for the implicit KASSERT(td_ucred !=
NULL)) everyhwere that td_ucred is used. :)   If it makes you feel better, put
it under INVARIANT_SUPPORT.  It is the same type of test though, as it is a way
of asserting that we aren't using td_ucred when a thread isn't in the kernel.

> On Fri, 8 Feb 2002, John Baldwin wrote:
> 
>> 
>> On 08-Feb-02 Julian Elischer wrote:
>> > 
>> > I'd like to commit the code to keep the ucred across userland,
>> > with the code to clear it to NULL kept under DEBUG ifdefs.
>> 
>> Use INVARIANTS for the ifdef macro name, but sure.
>> 
>> -- 
>> 
>> John Baldwin <[EMAIL PROTECTED]>  <><  http://www.FreeBSD.org/~jhb/
>> "Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/
>> 
> 

-- 

John Baldwin <[EMAIL PROTECTED]>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



Re: cred stuff..

2002-02-09 Thread Bruce Evans

On Fri, 8 Feb 2002, Julian Elischer wrote:

> I'd like to commit the code to keep the ucred across userland,
> with the code to clear it to NULL kept under DEBUG ifdefs.
>
> i.e.
>
> >  in trap(), ast() and syscall()
> >
> > if (td->td_ucred != p->p_ucred) {
> > PROC_LOCK(p);
> > if (td->td_ucred) {
> > crfree(td->td_ucred);
> > td->td_ucred = NULL;
> > }
> > if (p->p_ucred != NULL) {
> > td->td_ucred = crhold(p->p_ucred);
> > }
> > PROC_UNLOCK(p);
> > }

Please fix the style bugs in this before committing:
- explicit NULL in only one null pointer checks
- excessive braces for one of the ifs.

> and in userret() and ast()
>
> >#ifdef DEBUG  /*your choice of variable here*/
> > PROC_LOCK(p);
> > if (td->td_ucred) {
> > crfree(td->td_ucred);
> > td->td_ucred = NULL;
> > }
> > PROC_UNLOCK(p);
> >#endif

I think this is better left where it is in the functions that aquire
the locks.  It can then be done unconditionally, and not in a loop.

The style of the null pointer check in this is bug for bug compatible
with the corresponding one above.

Bruce


To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



RE: cred stuff..

2002-02-09 Thread Julian Elischer



On Fri, 8 Feb 2002, Julian Elischer wrote:

> John, (peter? others?)
> 
> How is it that getting a ucred reference is guarded by PROC_LOCK(p)
> but freeing it is guarded by mtx_lock(&Giant); 
> ?
> 
> Call me naive, but shouldn't they be guarded by the same thing?

In fact, if we had good atomic reference counting primatives, 
couldn't we just forget the locks entirely for this?

> 
> Julian
> 
> 
> On Fri, 8 Feb 2002, Julian Elischer wrote:
> 
> > I'm a little worried about invariants because the behaviour when 
> > INVARIANTS is set wil be different to teh behaviour when it is off, which
> > is 'strange' to say the least. Normally the behaviour si the same but you
> > just check for invariant conditions.
> > 
> > 
> > On Fri, 8 Feb 2002, John Baldwin wrote:
> > 
> > > 
> > > On 08-Feb-02 Julian Elischer wrote:
> > > > 
> > > > I'd like to commit the code to keep the ucred across userland,
> > > > with the code to clear it to NULL kept under DEBUG ifdefs.
> > > 
> > > Use INVARIANTS for the ifdef macro name, but sure.
> > > 
> > > -- 
> > > 
> > > John Baldwin <[EMAIL PROTECTED]>  <><  http://www.FreeBSD.org/~jhb/
> > > "Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/
> > > 
> > 
> > 
> > To Unsubscribe: send mail to [EMAIL PROTECTED]
> > with "unsubscribe freebsd-current" in the body of the message
> > 
> 
> 
> To Unsubscribe: send mail to [EMAIL PROTECTED]
> with "unsubscribe freebsd-current" in the body of the message
> 


To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



RE: cred stuff..

2002-02-08 Thread Julian Elischer

John, (peter? others?)

How is it that getting a ucred reference is guarded by PROC_LOCK(p)
but freeing it is guarded by mtx_lock(&Giant); 
?

Call me naive, but shouldn't they be guarded by the same thing?

Julian


On Fri, 8 Feb 2002, Julian Elischer wrote:

> I'm a little worried about invariants because the behaviour when 
> INVARIANTS is set wil be different to teh behaviour when it is off, which
> is 'strange' to say the least. Normally the behaviour si the same but you
> just check for invariant conditions.
> 
> 
> On Fri, 8 Feb 2002, John Baldwin wrote:
> 
> > 
> > On 08-Feb-02 Julian Elischer wrote:
> > > 
> > > I'd like to commit the code to keep the ucred across userland,
> > > with the code to clear it to NULL kept under DEBUG ifdefs.
> > 
> > Use INVARIANTS for the ifdef macro name, but sure.
> > 
> > -- 
> > 
> > John Baldwin <[EMAIL PROTECTED]>  <><  http://www.FreeBSD.org/~jhb/
> > "Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/
> > 
> 
> 
> To Unsubscribe: send mail to [EMAIL PROTECTED]
> with "unsubscribe freebsd-current" in the body of the message
> 


To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



RE: cred stuff..

2002-02-08 Thread Julian Elischer

I'm a little worried about invariants because the behaviour when 
INVARIANTS is set wil be different to teh behaviour when it is off, which
is 'strange' to say the least. Normally the behaviour si the same but you
just check for invariant conditions.


On Fri, 8 Feb 2002, John Baldwin wrote:

> 
> On 08-Feb-02 Julian Elischer wrote:
> > 
> > I'd like to commit the code to keep the ucred across userland,
> > with the code to clear it to NULL kept under DEBUG ifdefs.
> 
> Use INVARIANTS for the ifdef macro name, but sure.
> 
> -- 
> 
> John Baldwin <[EMAIL PROTECTED]>  <><  http://www.FreeBSD.org/~jhb/
> "Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/
> 


To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



RE: cred stuff..

2002-02-08 Thread John Baldwin


On 08-Feb-02 Julian Elischer wrote:
> 
> I'd like to commit the code to keep the ucred across userland,
> with the code to clear it to NULL kept under DEBUG ifdefs.

Use INVARIANTS for the ifdef macro name, but sure.

-- 

John Baldwin <[EMAIL PROTECTED]>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



cred stuff..

2002-02-08 Thread Julian Elischer


I'd like to commit the code to keep the ucred across userland,
with the code to clear it to NULL kept under DEBUG ifdefs.


i.e.

>  in trap(), ast() and syscall()
>
> if (td->td_ucred != p->p_ucred) {
> PROC_LOCK(p);
> if (td->td_ucred) {
> crfree(td->td_ucred);
> td->td_ucred = NULL;
> }
> if (p->p_ucred != NULL) {
> td->td_ucred = crhold(p->p_ucred);
> }
> PROC_UNLOCK(p);
> }

and in userret() and ast()

>#ifdef DEBUG  /*your choice of variable here*/
> PROC_LOCK(p);
> if (td->td_ucred) {
> crfree(td->td_ucred);
> td->td_ucred = NULL; 
> }
> PROC_UNLOCK(p);
>#endif








To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message