Date: Sat, 8 Nov 2008 16:07:50 +0200 From: Kostik Belousov <kostikbel@gmail.com> To: Attilio Rao <attilio@freebsd.org> Cc: svn-src-head@freebsd.org, Alexander Motin <mav@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org Subject: Re: svn commit: r184762 - head/sys/netgraph Message-ID: <20081108140750.GI18100@deviant.kiev.zoral.com.ua> In-Reply-To: <3bbf2fe10811080513x2b8bd201gcf24562360374494@mail.gmail.com> References: <200811080625.mA86Pvhw003486@svn.freebsd.org> <3bbf2fe10811080513x2b8bd201gcf24562360374494@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] On Sat, Nov 08, 2008 at 02:13:46PM +0100, Attilio Rao wrote: > 2008/11/8, Alexander Motin <mav@freebsd.org>: > > Author: mav > > Date: Sat Nov 8 06:25:57 2008 > > New Revision: 184762 > > URL: http://svn.freebsd.org/changeset/base/184762 > > > > Log: > > Don't use curthread to resolve file descriptor. Request may be queued, so > > thread will be different. Instead require sender to send process ID > > together with file descriptor. > > > > Modified: > > head/sys/netgraph/ng_tty.c > > > > Modified: head/sys/netgraph/ng_tty.c > > ============================================================================== > > --- head/sys/netgraph/ng_tty.c Sat Nov 8 04:43:24 2008 (r184761) > > +++ head/sys/netgraph/ng_tty.c Sat Nov 8 06:25:57 2008 (r184762) > > @@ -73,6 +73,7 @@ > > #include <sys/syslog.h> > > #include <sys/tty.h> > > #include <sys/ttycom.h> > > +#include <sys/proc.h> > > > > #include <net/if.h> > > #include <net/if_var.h> > > @@ -250,7 +251,8 @@ ngt_shutdown(node_p node) > > static int > > ngt_rcvmsg(node_p node, item_p item, hook_p lasthook) > > { > > - struct thread *td = curthread; /* XXX */ > > + struct proc *p; > > + struct thread *td; > > const sc_p sc = NG_NODE_PRIVATE(node); > > struct ng_mesg *msg, *resp = NULL; > > int error = 0; > > @@ -262,8 +264,14 @@ ngt_rcvmsg(node_p node, item_p item, hoo > > case NGM_TTY_SET_TTY: > > if (sc->tp != NULL) > > return (EBUSY); > > - error = ttyhook_register(&sc->tp, td, *(int *)msg->data, > > + > > + p = pfind(((int *)msg->data)[0]); > > + if (p == NULL) > > + return (ESRCH); > > + td = FIRST_THREAD_IN_PROC(p); > > + error = ttyhook_register(&sc->tp, td, ((int *)msg->data)[1], > > &ngt_hook, sc); > > + PROC_UNLOCK(p); > > if (error != 0) > > return (error); > > break; > > The threads iterator in strcut proc should be proc_slock protected, so > you need to grab/release it around FIRST_THREAD_IN_PROC(). I do not believe that process slock is needed there, since code just dereference a pointer. The process lock seems to be taken to guarantee that td is alive. On the other hand, ttyhook_register locks filedesc sx, that causes sleepable lock acquition after non-sleepable. I believe this is an actual problem with the change. Probably, this is even the problem with KPI of ttyhook_register, since caller need to somehow protect td from going away, and proc lock is a right lock to held. [-- Attachment #2 --] -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (FreeBSD) iEYEARECAAYFAkkVnTYACgkQC3+MBN1Mb4iSpACgicliFnkeSuGSowkg1mkmfg7Z tVUAn3A4KqCxgaQafJj5XDS9wXCAzKcY =+bB6 -----END PGP SIGNATURE-----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20081108140750.GI18100>