Re: lock reversal in fdalloc()
Alfred Perlstein wrote: > There's a bunch of bogosity in the ordering of allocation of > slots in the filedesc versus filling out the struct file and > insertion into the list that I need to work out. I should be > able to take a swipe at it in a couple of weeks hopefully. It's very tempting to put an API to it, and make everyone use it the same way, to guarantee consistency. It's also very tempting to hack the crap out of it to seperate file access itself from the system call layer itself, so that file slot allocation is something that is system call layer specific, which would greatly enhance the ability to do kernel level file I/O and other things. AIX has a nice model, here. Doing that would put the API barrier at the system call layer along, which would clean up almost everything that needed to use it. -- Terry To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: lock reversal in fdalloc()
* Terry Lambert <[EMAIL PROTECTED]> [020201 17:02] wrote: > Bruce Evans wrote: > > > basically it seems to get pissy if it doesn't get the file slot it asks > > > for, so if another thread wins the race here, we'll panic. this problem > > > seems to also exist for 4.x and previous versions of freebsd. > > > > > > I'd like to get this fixed. Any suggestions? I think simply > > > removing the assertion should remove this hazzard, correct? > > I think if you delayed the allocation, it'd be OK, but I don't > see a clean way to do it without a bit of work. > > > Something like that. This was apparently missed when the retry loop was > > added. Lite2 has the panic but not the retry loop. > > > > BTW, the retry loop also picks up changes to the limit on descriptors. > > In fdalloc(), the corresponding limit is treated as a loop invariant, > > but it is not invariant. I think the process's rlimit can't change, > > but maxfilesperproc can change even if the process doesn't block, since > > it is not protected by FILEDESC_LOCK() :-(. Fortunately, the > > maxfilesperproc limit isn't very important. > > I think this is a requirement. > > The problem is the case where the maxfilesperproc has *not* > changed, and some other thread wins the race to the last one, > you have to honor that, and fail the current attempt. > > Retries are fugly. 8-(. There's a bunch of bogosity in the ordering of allocation of slots in the filedesc versus filling out the struct file and insertion into the list that I need to work out. I should be able to take a swipe at it in a couple of weeks hopefully. -- -Alfred Perlstein [[EMAIL PROTECTED]] 'Instead of asking why a piece of software is using "1970s technology," start asking why software is ignoring 30 years of accumulated wisdom.' Tax deductable donations for FreeBSD: http://www.freebsdfoundation.org/ To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: lock reversal in fdalloc()
Bruce Evans wrote: > > basically it seems to get pissy if it doesn't get the file slot it asks > > for, so if another thread wins the race here, we'll panic. this problem > > seems to also exist for 4.x and previous versions of freebsd. > > > > I'd like to get this fixed. Any suggestions? I think simply > > removing the assertion should remove this hazzard, correct? I think if you delayed the allocation, it'd be OK, but I don't see a clean way to do it without a bit of work. > Something like that. This was apparently missed when the retry loop was > added. Lite2 has the panic but not the retry loop. > > BTW, the retry loop also picks up changes to the limit on descriptors. > In fdalloc(), the corresponding limit is treated as a loop invariant, > but it is not invariant. I think the process's rlimit can't change, > but maxfilesperproc can change even if the process doesn't block, since > it is not protected by FILEDESC_LOCK() :-(. Fortunately, the > maxfilesperproc limit isn't very important. I think this is a requirement. The problem is the case where the maxfilesperproc has *not* changed, and some other thread wins the race to the last one, you have to honor that, and fail the current attempt. Retries are fugly. 8-(. -- Terry To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: lock reversal in fdalloc()
On Fri, 1 Feb 2002, Alfred Perlstein wrote:
> It's actually safe... however dup2 thinks that we won't race
> for the file slot:
>
> if (new >= fdp->fd_nfiles) {
> if ((error = fdalloc(td, new, &i))) {
> FILEDESC_UNLOCK(fdp);
> return (error);
> }
> if (new != i)
> panic("dup2: fdalloc");
> /*
> * fdalloc() may block, retest everything.
> */
> goto retry;
> }
>
> basically it seems to get pissy if it doesn't get the file slot it asks
> for, so if another thread wins the race here, we'll panic. this problem
> seems to also exist for 4.x and previous versions of freebsd.
>
> I'd like to get this fixed. Any suggestions? I think simply
> removing the assertion should remove this hazzard, correct?
Something like that. This was apparently missed when the retry loop was
added. Lite2 has the panic but not the retry loop.
BTW, the retry loop also picks up changes to the limit on descriptors.
In fdalloc(), the corresponding limit is treated as a loop invariant,
but it is not invariant. I think the process's rlimit can't change,
but maxfilesperproc can change even if the process doesn't block, since
it is not protected by FILEDESC_LOCK() :-(. Fortunately, the
maxfilesperproc limit isn't very important.
Bruce
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
Re: lock reversal in fdalloc()
* Alfred Perlstein <[EMAIL PROTECTED]> [020131 10:48] wrote:
> * Bruce Evans <[EMAIL PROTECTED]> [020131 09:42] wrote:
> >
> > I'm not sure that releasing the lock here is safe, but other parts of
> > fdalloc() do this.
>
> I don't think this is safe at a glance, I think it's only safe right
> before return'ing from the function. I'll look at it later tonight.
It's actually safe... however dup2 thinks that we won't race
for the file slot:
if (new >= fdp->fd_nfiles) {
if ((error = fdalloc(td, new, &i))) {
FILEDESC_UNLOCK(fdp);
return (error);
}
if (new != i)
panic("dup2: fdalloc");
/*
* fdalloc() may block, retest everything.
*/
goto retry;
}
basically it seems to get pissy if it doesn't get the file slot it asks
for, so if another thread wins the race here, we'll panic. this problem
seems to also exist for 4.x and previous versions of freebsd.
I'd like to get this fixed. Any suggestions? I think simply
removing the assertion should remove this hazzard, correct?
--
-Alfred Perlstein [[EMAIL PROTECTED]]
'Instead of asking why a piece of software is using "1970s technology,"
start asking why software is ignoring 30 years of accumulated wisdom.'
Tax deductable donations for FreeBSD: http://www.freebsdfoundation.org/
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
Re: lock reversal in fdalloc()
* Bruce Evans <[EMAIL PROTECTED]> [020131 09:42] wrote:
> Jan 31 18:27:29 gamplex kernel: lock order reversal
> Jan 31 18:27:29 gamplex kernel: 1st 0xc26ea034 filedesc structure @
>./@/kern/kern_descrip.c:925
> Jan 31 18:27:29 gamplex kernel: 2nd 0xc031eca0 Giant @ ./@/kern/kern_descrip.c:959
>
> %%%
> Index: kern_descrip.c
> ===
> RCS file: /home/ncvs/src/sys/kern/kern_descrip.c,v
> retrieving revision 1.122
> diff -u -2 -r1.122 kern_descrip.c
> --- kern_descrip.c29 Jan 2002 22:54:19 - 1.122
> +++ kern_descrip.c31 Jan 2002 07:32:43 -
> @@ -957,7 +967,9 @@
> fdexpand++;
> if (oldofile != NULL) {
> + FILEDESC_UNLOCK(fdp);
> mtx_lock(&Giant);
> FREE(oldofile, M_FILEDESC);
> mtx_unlock(&Giant);
> + FILEDESC_LOCK(fdp);
> }
> }
> %%%
>
> I'm not sure that releasing the lock here is safe, but other parts of
> fdalloc() do this.
I don't think this is safe at a glance, I think it's only safe right
before return'ing from the function. I'll look at it later tonight.
-Alfred
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message
