From owner-freebsd-current@FreeBSD.ORG Fri Aug 31 13:37:36 2012 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 3D9D1106564A; Fri, 31 Aug 2012 13:37:36 +0000 (UTC) (envelope-from simon@comsys.ntu-kpi.kiev.ua) Received: from comsys.kpi.ua (comsys.kpi.ua [77.47.192.42]) by mx1.freebsd.org (Postfix) with ESMTP id A715E8FC15; Fri, 31 Aug 2012 13:37:35 +0000 (UTC) Received: from pm513-1.comsys.kpi.ua ([10.18.52.101] helo=pm513-1.comsys.ntu-kpi.kiev.ua) by comsys.kpi.ua with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.63) (envelope-from ) id 1T7RPi-0005AQ-7K; Fri, 31 Aug 2012 16:37:34 +0300 Received: by pm513-1.comsys.ntu-kpi.kiev.ua (Postfix, from userid 1001) id 8A2C21CC23; Fri, 31 Aug 2012 16:37:34 +0300 (EEST) Date: Fri, 31 Aug 2012 16:37:34 +0300 From: Andrey Simonenko To: John Baldwin Message-ID: <20120831133734.GA45044@pm513-1.comsys.ntu-kpi.kiev.ua> References: <503ED6A1.1060902@FreeBSD.org> <20120831110653.GA21089@pm513-1.comsys.ntu-kpi.kiev.ua> <201208310812.09902.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201208310812.09902.jhb@freebsd.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-Authenticated-User: simon@comsys.ntu-kpi.kiev.ua X-Authenticator: plain X-Sender-Verify: SUCCEEDED (sender exists & accepts mail) X-Exim-Version: 4.63 (build at 28-Apr-2011 07:11:12) X-Date: 2012-08-31 16:37:34 X-Connected-IP: 10.18.52.101:21219 X-Message-Linecount: 69 X-Body-Linecount: 50 X-Message-Size: 3234 X-Body-Size: 2367 Cc: Garrett Cooper , freebsd-current@freebsd.org, Pedro Giffuni Subject: Re: [CFT] Some updates to libc/rpc (second try) X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 31 Aug 2012 13:37:36 -0000 On Fri, Aug 31, 2012 at 08:12:09AM -0400, John Baldwin wrote: > On Friday, August 31, 2012 7:06:53 am Andrey Simonenko wrote: > > On Thu, Aug 30, 2012 at 02:37:17AM -0700, Garrett Cooper wrote: > > > > Detailed description of mistakes in these files and correct > implementation: > > > > > > > > http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/165710 > > > > > > A developer at $work (Isilon) developed a slightly simpler patch than > > > that based on our custom 7.x sources recently to deal with concurrency > > > issues in netconfig. I'll talk with a couple people to see whether or > > > not the solution can be contributed back [after some polishing -- > > > maybe -- and further testing]. > > > > Can you post changes and corrected bugs in getnetconfig.c and getnetpath.c > > in your (your $work) implementation. > > There is a thread on threads@ with patches to make the API thread-safe > that I believe came from Isilon. It was posted in the last week or so, > so it should be easy to find in the archives. > Thank you for this information. I checked the logic of their changes. Making all data per-thread is wrong from my point of view. 1. Several threads can call getnetconfig() using the same handler obtained by setnetconfig(). If each thread has own FILE pointer, then some getnetconfig() will crash a program since fgets() will be called for NULL FILE pointer. 2. One thread can get handler by setnetconfig() and pass this handler to another thread and getnetconfig() will crash a program. This is the similar mistake, but getnetconfig() is not called in parallel. 3. Each thread has to open netconfig(5) database, parse its content every time for each getnetconfig() call since data is not cached. This is slow. There is one per-thread value, it is error code of the last failed function, this value cannot be kept in handler, since nc_perror() and nc_sperror() do not have handler argument. Since printing error is expected in the same thread when getnetconfig() failed (for example), I think this is Ok. If error is print in another thread, then we simply will not see correct error message, but program will not be crashed. I just commented only per-thread idea of data in getnetconfig.c, I do not compare my implementation, since their patch does not correct any other mistake described and corrected in my PR.