From owner-svn-src-head@FreeBSD.ORG Sat Nov 8 14:47:43 2008 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id EF5B61065670; Sat, 8 Nov 2008 14:47:42 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: from fg-out-1718.google.com (fg-out-1718.google.com [72.14.220.156]) by mx1.freebsd.org (Postfix) with ESMTP id EBF398FC16; Sat, 8 Nov 2008 14:47:41 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: by fg-out-1718.google.com with SMTP id l26so1406086fgb.35 for ; Sat, 08 Nov 2008 06:47:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:message-id:date:from:sender :to:subject:cc:in-reply-to:mime-version:content-type :content-transfer-encoding:content-disposition:references :x-google-sender-auth; bh=EYNvg8wFf75ix5yoqg1s5XxDVDScAZcn7C6suY7CtK0=; b=cq+jOM80Op+5pYwQTfk+BfUfdDR5HoRW7nEGMn9EEcDIg08yrVu1WvO5j9hr+46T16 jD08cE+qDIAvDrO7elHlRyusF1O7rKxlC56ow0T1tL2GnLnFlOKG0+VMpxtzmknNLEgt vsm+Xu+hvvSIH/bz+1VHzHglYg498BZF6u070= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:sender:to:subject:cc:in-reply-to:mime-version :content-type:content-transfer-encoding:content-disposition :references:x-google-sender-auth; b=Y6FIvqD9kG7UX2GvdKNy96Y6W7vvn1k6U8axk9FOpb1scmTdOkR+YoUA+Sd5uBG7jB slljWNl8QHqZ8YGTAHoTkhb2buyz/enRkVuHn07K+MBqv86gEQGMOyv2RXCk8hEpTWIm umepke1kPyGLWVhpeoXXczG60V8gnpMiAX+n8= Received: by 10.86.62.3 with SMTP id k3mr5130726fga.6.1226155660728; Sat, 08 Nov 2008 06:47:40 -0800 (PST) Received: by 10.86.2.18 with HTTP; Sat, 8 Nov 2008 06:47:40 -0800 (PST) Message-ID: <3bbf2fe10811080647n3f43c0d5k19a6e673c34b868c@mail.gmail.com> Date: Sat, 8 Nov 2008 15:47:40 +0100 From: "Attilio Rao" Sender: asmrookie@gmail.com To: "Kostik Belousov" In-Reply-To: <20081108140750.GI18100@deviant.kiev.zoral.com.ua> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <200811080625.mA86Pvhw003486@svn.freebsd.org> <3bbf2fe10811080513x2b8bd201gcf24562360374494@mail.gmail.com> <20081108140750.GI18100@deviant.kiev.zoral.com.ua> X-Google-Sender-Auth: dc0dd88b6809d427 Cc: svn-src-head@freebsd.org, Alexander Motin , src-committers@freebsd.org, svn-src-all@freebsd.org Subject: Re: svn commit: r184762 - head/sys/netgraph X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 08 Nov 2008 14:47:43 -0000 2008/11/8, Kostik Belousov : > On Sat, Nov 08, 2008 at 02:13:46PM +0100, Attilio Rao wrote: > > 2008/11/8, Alexander Motin : > > > 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 > > > #include > > > #include > > > +#include > > > > > > #include > > > #include > > > @@ -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. I spoke briefly with Kostik on IRC and the problem he exposes is real. He suggested to use directly a struct file object to be passed and it seems fine by me. Also, as you are not interested in what thread is returned as first, you probabilly don't need the proc_slock. Thanks, Attilio -- Peace can only be achieved by understanding - A. Einstein