Date: Sat, 12 Jan 2002 19:16:45 -0800 From: Alfred Perlstein <bright@mu.org> To: John Baldwin <jhb@FreeBSD.org> Cc: smp@freebsd.org, dillon@freebsd.org, tanimura@freebsd.org Subject: Re: fd locking. Message-ID: <20020112191645.J7984@elvis.mu.org> In-Reply-To: <XFMail.020112185945.jhb@FreeBSD.org>; from jhb@FreeBSD.org on Sat, Jan 12, 2002 at 06:59:45PM -0800 References: <XFMail.020112174456.jhb@FreeBSD.org> <XFMail.020112185945.jhb@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
* John Baldwin <jhb@FreeBSD.org> [020112 19:00] wrote: > > On 13-Jan-02 John Baldwin wrote: > > > > On 13-Jan-02 Alfred Perlstein wrote: > >> * Alfred Perlstein <bright@mu.org> [020112 03:11] wrote: > >>> I've got world building with these patches. > >>> > >>> http://people.freebsd.org/~alfred/fd.diff > >>> > >>> or > >>> > >>> http://people.freebsd.org/~alfred/fd.diff.gz > > Why did you add the else here: Axed. > and another proc -> thread bug at the bottom of this hunk: ok. > > And another one: > > diff -u -r1.34 svr4_misc.c yup. > hmm, some more, have you compiled this? :) Might want to compile LINT (minus > usb since that is broken) and fix the new warnings I've compiled my kernel, i'm in the process of building world, but it's kinda slow because i have all the INVARIANT and WITNESS stuff on for that. > In svr4_stream.c, do_putmsg and do_getmsg should be taking a thread not a proc > as their first argument and callers should be fixed as well. Can this be delayed? > You might consider a file_init() and file_destroy() function or macro like so: > > void > file_init(struct file *fp) > { > > mtx_init(&fp->f_mtx, "struct file", MTX_DEF); > fp->f_count = 1; > } > > void > file_destroy(struct file *fp) > { > > mtx_destroy(&fp->f_mtx); > } > > I would just make file_destroy a macro for now. Having file_init a function > would save a little space since the "struct file" string wouldn't be > duplicated, but you can always change it later. You could add more stuff if > needed as well. :) Can this be delayed? > > Hmm, do you think you could change fdalloc() to take a filedesc * instead of a > thread so it's clearer when you lock the old filedesc that it is being used? Might work, Can this be delayed? > hmm, for this code: > > + mtx_init(&fp->f_mtx, "file structure", MTX_DEF); > + fp->f_gcflag = 0; > fp->f_count = 1; > fp->f_cred = crhold(p->p_ucred); > fp->f_ops = &badfileops; > fp->f_seqcount = 1; > + FILEDESC_UNLOCK(p->p_fd); > + sx_xlock(&filelist_lock); > + FILEDESC_LOCK(p->p_fd); > if ((fq = p->p_fd->fd_ofiles[0])) { > LIST_INSERT_AFTER(fq, fp, f_list); > } else { > LIST_INSERT_HEAD(&filehead, fp, f_list); > } > p->p_fd->fd_ofiles[i] = fp; > + FILEDESC_UNLOCK(p->p_fd); > + sx_xunlock(&filelist_lock); > if (resultfp) > *resultfp = fp; > if (resultfd) > > You could xlock filelist_lock earlier before the first FILEDESC_LOCK with > associated changes to avoid as many locking operations. You wouldn't keep the > xlock held for much longer and it would probably be quicker in the long run. Yes, but those codes call malloc with M_WAITOK, if someone was to close a filedescriptor i may get deadlock because they block on the filehead sx lock while i'm blocked in malloc and i already have the filedesc lock. > Bruce is going to not like you for adding nested includes of sys/lock.h and > sys/mutex.h. Instead, add nested includes of sys/_lock.h and sys/_mutex.h, and > then add sys/lock.h and sys/mutex.h to the files that need them. Can this be delayed? > Other then that it looks great. Can you clean these bits up and post a new > patch for folks to test. Aside form svr4, the current patch should be good for > testing as well. Esp. need people with SMP machines to test this stuff. New patch up: http://people.freebsd.org/~alfred/fd.diff I still need to fix Matt's fd holding stuff, but I'm anxious to get this in before I loose it all to some massive structure renaming or whitespace run like I have before. :) -- -Alfred Perlstein [alfred@freebsd.org] '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 majordomo@FreeBSD.org with "unsubscribe freebsd-smp" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020112191645.J7984>