From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS3215 2.6.0.0/16 X-Spam-Status: No, score=-3.7 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id C8EA41F8C6 for ; Wed, 14 Jul 2021 16:45:06 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id CED63398701C for ; Wed, 14 Jul 2021 16:45:05 +0000 (GMT) Received: from smtp4-g21.free.fr (smtp4-g21.free.fr [IPv6:2a01:e0c:1:1599::13]) by sourceware.org (Postfix) with ESMTPS id 5050D398643C; Wed, 14 Jul 2021 16:42:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 5050D398643C Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=morinfr.org Authentication-Results: sourceware.org; spf=none smtp.mailfrom=morinfr.org Received: from bender.morinfr.org (unknown [82.64.86.27]) by smtp4-g21.free.fr (Postfix) with ESMTPS id F3A5119F574; Wed, 14 Jul 2021 18:42:05 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=morinfr.org ; s=20170427; h=In-Reply-To:Content-Type:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=u4STSFeiOUiNNTHMpcrvzD4FngygzAB60jJmCbpFXSQ=; b=FvPb8fgEui2NS/dbzWAH9l9wwV 3M6RViZ6UPQVUN4L5B8ZDiqEoQ3tQ1lNe6f7w57eEBSyfevM/V3vuRJBweUXbTD3/gCQipFQOzGDz MYWjbtJcoWEz8J4DkkWGKKd4Ws7iaWn/GgVSKLEvRMj8ZOTimEMgqy05XCk//Ij5hRy8=; Received: from guillaum by bender.morinfr.org with local (Exim 4.92) (envelope-from ) id 1m3hxV-00035s-GJ; Wed, 14 Jul 2021 18:42:05 +0200 Date: Wed, 14 Jul 2021 18:42:05 +0200 From: Guillaume Morin To: Siddhesh Poyarekar Subject: Re: [PATCH v8 03/10] Remove __morecore and __default_morecore Message-ID: <20210714164204.GA1103@bender.morinfr.org> References: <20210713073845.504356-1-siddhesh@sourceware.org> <20210713073845.504356-4-siddhesh@sourceware.org> <9925362a-d278-a0ad-f504-ae08ca93f628@gotplt.org> <20210714125415.GA24678@bender.morinfr.org> <45ba79b4-02e6-fae5-ae9b-db6c8a01aecb@gotplt.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <45ba79b4-02e6-fae5-ae9b-db6c8a01aecb@gotplt.org> User-Agent: Mutt/1.10.1 (2018-07-13) X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: fweimer@redhat.com, Siddhesh Poyarekar , Guillaume Morin , libc-alpha@sourceware.org Errors-To: libc-alpha-bounces+e=80x24.org@sourceware.org Sender: "Libc-alpha" On 14 Jul 19:43, Siddhesh Poyarekar wrote: > > But basically, this breaks https://github.com/libhugetlbfs/libhugetlbfs/ > > which is old (started in 2006) and commonly used library. The library > > is plugging its own morecore() implementation to use hugetlb pages to > > back the malloc heap. > Thanks, AFAICT, libhugetlbfs only uses __morecore and not the rest of the > interfaces. The thing is, __morecore and friends have been deprecated for a > year and building anything with __morecore ought to give the deprecation > warning. Probably. But you surely know that most people get glibc from their distributions and do not build their own glibc. My main box is running Debian stable for example. The next release (bullseye) seems like it'll be shipping 2.31 (where it's not deprecated yet, I believe). RHEL8 ships with a libhugetlbfs rpm (so users do not need to build it at all unlike Debian), which will become useless with this change! Most users will never have seen the deprecation warning before they see the actual removal. That said, I was personally aware since it was pointed out on a libhugetlbfs github issue that it was deprecated in 2.32. I think the hope was it would be replaced by something usable, and users would not be left with *no* solution (I was also quietly hoping Eric Munson, the libhugetlbfs maintainer would be reaching out). But if you remove it, I am not quite sure what you expect libhugetlbfs users to do, really. > I don't think we would like to export a different interface to do the same > thing because it still doesn't allow us to clamp down further on the system > malloc implementation. Besides, the morecore hooking interface is severely > limited because it only affects the main arena. Non-main arenas have their > own allocation techniques and it doesn't really make sense to have an > interface to control just the main arena. Well, I do not see why morecore() does not make sense as a plugin. I am not seeing any technical argument supporting that it will prevent clamping down the implementation provided you document the currently acceptable semantics for this interface. libhugetlbfs users are aware this only works in the main arena. I certainly agree this is not ideal and this is definitely something that could be fixed as well. But again, keep in mind people have been using this for 15+ years. Obviously, if you're arguing to extend the glibc hookability so it works for all arenas, I am all ears, certainly would help out and would be a day one user :-) > There is only the one known issue that you reported but that may well be > because the interface isn't well tested. There have been many changes to > malloc over the years and I cannot say for certain that the idea that > morecore could be anything other than brk has been consistently considered. > We don't have tests to verify that either. I guess you mean sbrk() above and not brk(). Again, you seem to be ignoring the 15 years of people using libhugetblfs. It's been tested extensively by users. I certainly agree it should get testing in the glibc testsuite, but it's actually trivial afaict: - morecore() in effect has very simple semantics. It takes a size and returns a pointer to the beginning of the new area. The only thing that differs here from sbrk() is the fact that a new area might not be contiguous with the previous segment. But that's supported in the malloc code, otherwise libhugetlbfs morecore() would never have worked (that's the MORECORE_CONTIGUOUS handling in the malloc code). That's really _all_ you need to test. And I would presume non contiguous heaps are implemented not just for morecore() sake (not sure though). If that's case, no actual specific testing needs to be done. - If you _only_ allow morecore interposition through a DSO (as I was suggesting), you do _not_ even need to support non contiguous heaps (at least for morecore()) since at that point it becomes guaranteed that morecore() will always point to the same implementation. Then there is _nothing_ to test at that point. Proper morecore() implementations can semantically behave exactly like sbrk! - A morecore2() hook that takes the old top of the heap as well as the size would also allow libhugetlbfs() to implement a perfectly continuous heap and so would semantically behave like sbrk(). But it does not look like it's being considered here :-) > Further, morecore limits its influence to the main arena and it is > possible that future changes could break this in ways that we cannot > anticipate today. As I said, we're aware and it not ideal :-). But if you're using single threaded software, that covers all allocations under the mmap threshold (which is tunable). So it's _very_ easy to make this hook usable for a lot of software. The limitations are not such a big deal (but again, I'd prefer if they were not there). > Basically this is technical debt we've accumulated from the days when malloc > assumed single threaded programs and was incrementally hacked to add in > multi-threaded support. We'd like to clean this up. Honestly from my PoV, some usual feature was added a while ago. The rest of the code was changed without paying attention to the feature (bug report ignored for 5 years as well). And you're using this as an argument to remove it.... I don't think this is tech debt at all, the feature is simply paying the price of being ignored. It does _seem_ to me that little effort have been made to see if it was workable at all. > The malloc code in glibc is quite general purpose and has little dependency > on libc internals. It may well be possible for you to copy the source files > over into libhugetlbfs to implement the interposer within libhugetlbfs that > does exactly what you need. In fact, that may even allow you to extend > hugetlbfs support to non-main arenas. To be clear, I am not the libhugetblfs maintainer, just a user (that investigated the trimming issue). I think everybody is aware they can fork the glibc malloc implementation. Surely you realize that it's not ideal but even feasible for most users. Guillaume. -- Guillaume Morin