From owner-svn-src-head@FreeBSD.ORG Sun Sep 28 21:49:05 2014 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id A2C67FB1; Sun, 28 Sep 2014 21:49:05 +0000 (UTC) Received: from mail-qg0-x230.google.com (mail-qg0-x230.google.com [IPv6:2607:f8b0:400d:c04::230]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 2F185C8F; Sun, 28 Sep 2014 21:49:05 +0000 (UTC) Received: by mail-qg0-f48.google.com with SMTP id i50so1048770qgf.21 for ; Sun, 28 Sep 2014 14:49:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=CFQRoNm59wG646LQiZIILY+/sB5D+wy0HnilXeBZU/k=; b=A0374GitBXdRc5slSF/iJEmiHhIOxFHpMiOPtIv/+EsnwcwW3Wv/k5eDkaS/JhgDcj wkyJWYFXzEMlrF5DjYCtK//Sk56f23abpk3ppQTyYuG1xihq4iYz4u+/yE6Dpfx+xY2i rq0+xsnneBopzJwVF9MUJb9esITJmHNyyIpHxC1GQZ2gZGtGUlHnWB0GZ+96uMIq9dDQ vpAdamsp3/NqAZwA28r+2xrUaPMg3qp88yFiRsUFu1GbUjow3HtUdP7Ududou7lA26Pe YKRdGgud8HiW3XGZqVf7+g7vcL0hj+dq7GdKQl5HDZ40kIXOyFQfllBFn5YeE0zvPa5h 9pAw== MIME-Version: 1.0 X-Received: by 10.140.28.8 with SMTP id 8mr55216837qgy.19.1411940944231; Sun, 28 Sep 2014 14:49:04 -0700 (PDT) Received: by 10.140.97.100 with HTTP; Sun, 28 Sep 2014 14:49:04 -0700 (PDT) In-Reply-To: References: <201409282112.s8SLCNuA067731@svn.freebsd.org> Date: Sun, 28 Sep 2014 14:49:04 -0700 Message-ID: Subject: Re: svn commit: r272270 - head/sys/kern From: Neel Natu To: Garrett Cooper Content-Type: text/plain; charset=UTF-8 Cc: "svn-src-head@freebsd.org" , "svn-src-all@freebsd.org" , "src-committers@freebsd.org" , Neel Natu X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.18-1 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: Sun, 28 Sep 2014 21:49:05 -0000 Hi, On Sun, Sep 28, 2014 at 2:25 PM, Garrett Cooper wrote: > Hi Neel! > Thank you for fixing this. I think we've run into similar issues with customizations to the tty code at Isilon. I have one comment... > Thanks! > Happy to help. >> On Sep 28, 2014, at 14:12, Neel Natu wrote: >> >> Author: neel >> Date: Sun Sep 28 21:12:23 2014 >> New Revision: 272270 >> URL: http://svnweb.freebsd.org/changeset/base/272270 >> >> Log: >> tty_rel_free() can be called more than once for the same tty so make sure >> that the tty is dequeued from 'tty_list' only the first time. >> >> The panic below was seen when a revoke(2) was issued on an nmdm device. >> In this case there was also a thread that was blocked on a read(2) on the >> device. The revoke(2) woke up the blocked thread which would typically >> return an error to userspace. In this case the reader also held the last >> reference on the file descriptor so fdrop() ended up calling tty_rel_free() >> via ttydev_close(). >> >> tty_rel_free() then tried to dequeue 'tp' again which led to the panic. >> >> panic: Bad link elm 0xfffff80042602400 prev->next != elm >> cpuid = 1 >> KDB: stack backtrace: >> db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe00f9c90460 >> kdb_backtrace() at kdb_backtrace+0x39/frame 0xfffffe00f9c90510 >> vpanic() at vpanic+0x189/frame 0xfffffe00f9c90590 >> panic() at panic+0x43/frame 0xfffffe00f9c905f0 >> tty_rel_free() at tty_rel_free+0x29b/frame 0xfffffe00f9c90640 >> ttydev_close() at ttydev_close+0x1f9/frame 0xfffffe00f9c90690 >> devfs_close() at devfs_close+0x298/frame 0xfffffe00f9c90720 >> VOP_CLOSE_APV() at VOP_CLOSE_APV+0x13c/frame 0xfffffe00f9c90770 >> vn_close() at vn_close+0x194/frame 0xfffffe00f9c90810 >> vn_closefile() at vn_closefile+0x48/frame 0xfffffe00f9c90890 >> devfs_close_f() at devfs_close_f+0x2c/frame 0xfffffe00f9c908c0 >> _fdrop() at _fdrop+0x29/frame 0xfffffe00f9c908e0 >> sys_read() at sys_read+0x63/frame 0xfffffe00f9c90980 >> amd64_syscall() at amd64_syscall+0x2b3/frame 0xfffffe00f9c90ab0 >> Xfast_syscall() at Xfast_syscall+0xfb/frame 0xfffffe00f9c90ab0 >> --- syscall (3, FreeBSD ELF64, sys_read), rip = 0x800b78d8a, rsp = 0x7fffffbfdaf8, rbp = 0x7fffffbfdb30 --- >> >> CR: https://reviews.freebsd.org/D851 >> Reviewed by: glebius, ed >> Reported by: Leon Dang >> Sponsored by: Nahanni Systems >> MFC after: 1 week >> >> Modified: >> head/sys/kern/tty.c >> >> Modified: head/sys/kern/tty.c >> ============================================================================== >> --- head/sys/kern/tty.c Sun Sep 28 20:06:02 2014 (r272269) >> +++ head/sys/kern/tty.c Sun Sep 28 21:12:23 2014 (r272270) >> @@ -1055,13 +1055,13 @@ tty_rel_free(struct tty *tp) >> tp->t_dev = NULL; >> tty_unlock(tp); >> >> - sx_xlock(&tty_list_sx); >> - TAILQ_REMOVE(&tty_list, tp, t_list); >> - tty_list_count--; >> - sx_xunlock(&tty_list_sx); >> - >> - if (dev != NULL) >> + if (dev != NULL) { > > if (dev == NULL) > return; > > Would be result in less change in an MFC. > Yes, indeed. I don't think it makes any difference in readability though. best Neel >> + sx_xlock(&tty_list_sx); >> + TAILQ_REMOVE(&tty_list, tp, t_list); >> + tty_list_count--; >> + sx_xunlock(&tty_list_sx); >> destroy_dev_sched_cb(dev, tty_dealloc, tp); >> + } >> } >> >> void