From owner-freebsd-hackers@FreeBSD.ORG Thu Mar 8 14:42:49 2007 Return-Path: X-Original-To: freebsd-hackers@freebsd.org Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 8179E16A415 for ; Thu, 8 Mar 2007 14:42:49 +0000 (UTC) (envelope-from emaste@phaedrus.sandvine.ca) Received: from gw.sandvine.com (sandvine.com [199.243.201.138]) by mx1.freebsd.org (Postfix) with ESMTP id 3DA6313C4A6 for ; Thu, 8 Mar 2007 14:42:47 +0000 (UTC) (envelope-from emaste@phaedrus.sandvine.ca) Received: from labgw2.phaedrus.sandvine.com ([192.168.3.11]) by gw.sandvine.com with Microsoft SMTPSVC(6.0.3790.1830); Thu, 8 Mar 2007 09:42:46 -0500 Received: by labgw2.phaedrus.sandvine.com (Postfix, from userid 12627) id 38EAB11608; Thu, 8 Mar 2007 09:42:46 -0500 (EST) Date: Thu, 8 Mar 2007 09:42:46 -0500 From: Ed Maste To: Divacky Roman Message-ID: <20070308144245.GA69780@sandvine.com> References: <20070307230731.GA71684@sandvine.com> <20070308095522.GA14973@stud.fit.vutbr.cz> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20070308095522.GA14973@stud.fit.vutbr.cz> User-Agent: Mutt/1.4.2.1i X-OriginalArrivalTime: 08 Mar 2007 14:42:46.0412 (UTC) FILETIME=[096494C0:01C76190] Cc: freebsd-hackers@freebsd.org Subject: Re: Hung kernel from sysv semaphore semu_list corruption X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 Mar 2007 14:42:49 -0000 On Thu, Mar 08, 2007 at 10:55:22AM +0100, Divacky Roman wrote: > this is wrong.. you cannot remove element from a *LIST when its iterated using *LIST_FOREACH. > Use *LIST_FOREACH_SAFE instead... We're not freeing the item in the loop so it would work unless QUEUE_MACRO_DEBUG is turned on to intentionally trash the link pointer on _REMOVE. You're right though it's not the correct approach. There's an efficiency issue with the previous patch anyhow in that SLIST_REMOVE walks the list again to find the element to remove. How about the patch below instead: Index: sysv_sem.c =================================================================== RCS file: /usr/cvs/src/sys/kern/sysv_sem.c,v retrieving revision 1.85 diff -u -r1.85 sysv_sem.c --- sysv_sem.c 22 Oct 2006 11:52:13 -0000 1.85 +++ sysv_sem.c 8 Mar 2007 14:22:43 -0000 @@ -1301,8 +1301,10 @@ */ SEMUNDO_LOCK(); SLIST_FOREACH_PREVPTR(suptr, supptr, &semu_list, un_next) { - if (suptr->un_proc == p) + if (suptr->un_proc == p) { + *supptr = SLIST_NEXT(suptr, un_next); break; + } } SEMUNDO_UNLOCK(); @@ -1362,8 +1364,9 @@ * Deallocate the undo vector. */ DPRINTF(("removing vector\n")); + SEMUNDO_LOCK(); suptr->un_proc = NULL; - *supptr = SLIST_NEXT(suptr, un_next); + SEMUNDO_UNLOCK(); }