From owner-freebsd-arch@freebsd.org Tue Dec 29 19:18:35 2015 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id BA53DA55F3D; Tue, 29 Dec 2015 19:18:35 +0000 (UTC) (envelope-from deischen@freebsd.org) Received: from mail.netplex.net (mail.netplex.net [204.213.176.9]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "*.netplex.net", Issuer "RapidSSL SHA256 CA - G3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 913C91822; Tue, 29 Dec 2015 19:18:34 +0000 (UTC) (envelope-from deischen@freebsd.org) Received: from sea.ntplx.net (sea.ntplx.net [204.213.176.11]) by mail.netplex.net (8.15.1/8.15.1/NETPLEX) with ESMTP id tBTJIRG0015683; Tue, 29 Dec 2015 14:18:27 -0500 X-Virus-Scanned: by AMaViS and Clam AntiVirus (mail.netplex.net) X-Greylist: Message whitelisted by DRAC access database, not delayed by milter-greylist-4.4.3 (mail.netplex.net [204.213.176.9]); Tue, 29 Dec 2015 14:18:27 -0500 (EST) Date: Tue, 29 Dec 2015 14:18:27 -0500 (EST) From: Daniel Eischen X-X-Sender: eischen@sea.ntplx.net Reply-To: Daniel Eischen To: Konstantin Belousov cc: John Baldwin , freebsd-arch@freebsd.org, freebsd-threads@freebsd.org Subject: Re: libthr shared locks In-Reply-To: <20151229184405.GY3625@kib.kiev.ua> Message-ID: References: <5496837.TbTQtANDNj@ralph.baldwin.cx> <20151224191408.GA3625@kib.kiev.ua> <20151226105409.GH3625@kib.kiev.ua> <20151226234424.GJ3625@kib.kiev.ua> <20151228105157.GQ3625@kib.kiev.ua> <20151229184405.GY3625@kib.kiev.ua> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 29 Dec 2015 19:18:35 -0000 On Tue, 29 Dec 2015, Konstantin Belousov wrote: > On Mon, Dec 28, 2015 at 11:59:02AM -0500, Daniel Eischen wrote: >> On Mon, 28 Dec 2015, Konstantin Belousov wrote: >> >>> On Sun, Dec 27, 2015 at 11:44:44AM -0500, Daniel Eischen wrote: >>> Adding the self-pointer is good idea, but it would not work for the >>> shared locks. If you are going to work on this, then I will scratch >>> my patch, to not impede on your work. >> >> Shared locks would work, but only for the new ABI (I guess that >> would be FBSD1.4 in HEAD, or FBSD1.5 if we bumped it again just >> for this). > I only mean that self-pointer sometimes would cause additional issues. > >> >> Another option is to just increase the size for the pthread synch >> types without really changing the first element (still a pointer >> to the allocated object - the implementation would stay the same). >> The only breakage would then be storage layout issues for new >> compiles linking against older ones. For FreeBSD-12, we could >> change the implementation to a real inlining (without changing >> the storage size), that gives a major release cycle for 3rd >> parties to make the change. I'm not sure if this would really >> be much of a benefit - the storage size change alone would >> probably be the greatest pain. > It is a good intermediate point in the whole plan of changing the ABI. > The work required to support pshared after the size is increased is not > completely trivial, but is not a rocket science either. But your note > about intermediate step might give additional freedom in executing the > whole plan. > >> >>> Taking out the inlining bits from the David patch, or (which would I do, >>> if doing this) just reimplementing it from scratch is easy enough and >>> just require some time. I estimated this job to take between one and >>> two weeks. >> >> I think a lot of David's patch is the renaming of all the >> elements of the public structs to prepend '__'. I was thinking >> it would be nice to have the public structs be something like >> this: >> >> struct pthread_mutex_t { >> uint32_t __x[IMPL_REQ + IMPL_SPARE + pad_to_CACHE_LINE_SIZE]; >> }; >> >> and then have libthr override the definition. That would >> make declaring PTHREAD_MUTEX_INITIALIZER, etc, a little >> magical, but avoid a lot of needless churn in libthr. > This is very good suggestion, I fully agree. There are some more details, > e.g. it would be better to use uint64_t or explicit align attribute, to > get proper alignment, but overall idea is sound, of course. > >> >> But, I agree, just doing the inlining is basic grunt work >> and limited in duration. Your estimate seems reasonable. >> >>> What is really thrilling is to manage the consequences of the ABI >>> breakage. When I did the evaluation for the FF project, I put a six >>> months extent for the whole work. This is for things like looking at >>> the ports impact, trying to know in advance where mixed things start to >>> break, monitoring the users complains about issues caused by the ABI >>> break etc. >> >> There's not much we can really do to manage it, just make it >> known that all ports need to be recompiled on 11 in order to >> be on the safe side. The libmap.conf trick will not work >> either. > No. This is probably the point where our positions diverge most. > > I do think that some mitigation measures can be applied, e.g. your > self-pointer trick, check for presence of 'other' lib when loading > given threading library, renaming etc. It should be thought out in > advance. > > Another thing to do is to monitor mailing lists, react to the user > issues, see if it is related to the ABI breakage, and possibly create > another measures which would make some issues less painful. > > I consider both activities absolutely vital when doing this surgery. > In principle, I may do both, but it requires more planning and is > not immediately reachable comparing with the off-page approach to > only get pshared right now. > >>> You see my position, I would avoid ABI breakage at all costs. If I >>> cannot talk you against this, please do not consider the work done after >>> the inlining patch is committed to HEAD, it is actually only start >>> there. >> >> Ok, I do think we want to do it at some point. Perhaps 12-current >> would be better... > > Some people in private also told me that they consider 11 too risky target > for this change. We will see. Perhaps another option is doing all of what we said above and also use your suggestion of libthr2, but have both libthr and libthr2, where libthr2 is labeled experimental or something similar. Have a WITH_LIBTHR2 build flag that conditionally inlines the pthread locks and enables (links w/-pthread) libthr2 for FreeBSD base OS. A note/comment for the WITH_LIBTHR2 flag states that all ports must also be recompiled to be on the safe side. This allows one to switch back and forth, as long as any affected ports are also rebuilt. If we were to introduce this in a future 12-current, we might have the WITH_LIBTHR_FOO flag anyway, at least initially, so perhaps it wouldn't be extra work. -- DE