From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-1.mimecast.com (us-smtp-2.mimecast.com [205.139.110.61]) by sourceware.org (Postfix) with ESMTP id B87273851C07 for ; Tue, 12 May 2020 03:39:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org B87273851C07 Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-198-Rijvf6OqOsiEMbwL2EWwFg-1; Mon, 11 May 2020 23:39:55 -0400 X-MC-Unique: Rijvf6OqOsiEMbwL2EWwFg-1 Received: by mail-qt1-f199.google.com with SMTP id z8so12729460qtu.17 for ; Mon, 11 May 2020 20:39:55 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:organization :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=/e+JFCbrYfEZ5fY04tFilv5ey72vkRoB5FIBEEVVhso=; b=ZIPJOrwZc/iMSIQigANFBdjphGBiOVdgXvZgrE289MEs+70+Akfvue62MS9dY/z6je D9/QKicP0ps2MtRaWArFVs2hXU2c8Udqclmz4uTRGAo4HLudH5ULFp3rGWJDenN/RjJa EJlDaeuFzi8+7SWQXK/z6QxChQkNFE9xeDyGkiKlEDQhcVVptYZtlivP5jwTgcVMoLlQ hr9hstyZzsfuy3RTdW94qsZhcNqv+OfCDMsmWM3IJ8mGdLFKjCs9sRfowLMOv3XL1feu fj/ObRg72UnysRFxV+2X6/8MD73hK8JknTqeai7fbnUM+S9aK4WB0FV3bJVUkCIOkNYq kzqA== X-Gm-Message-State: AGi0PuZ23pz5M0uYHVw/xoO49RgCUxwAerCkrHJaWdSk1fthEKqL3byu QGe+WzGnFvexxYNkxUkREWII4wy9eKlfGyD86gN070bqDZKZetUpwc/dfKIypR2W1dTH4ZXdV77 lxZ7C9ksW8BY4lcB2jdRF X-Received: by 2002:a37:6dc4:: with SMTP id i187mr19134579qkc.358.1589254795026; Mon, 11 May 2020 20:39:55 -0700 (PDT) X-Google-Smtp-Source: APiQypJRixunsJdE+Z53swxU5xRJM722OtSUtNys+J5tTXPZ22q2PfrwQtyZdAf/nYq0qXkjHBcpjw== X-Received: by 2002:a37:6dc4:: with SMTP id i187mr19134543qkc.358.1589254794131; Mon, 11 May 2020 20:39:54 -0700 (PDT) Received: from [192.168.1.4] (198-84-170-103.cpe.teksavvy.com. [198.84.170.103]) by smtp.gmail.com with ESMTPSA id o31sm6572268qto.64.2020.05.11.20.39.53 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 11 May 2020 20:39:53 -0700 (PDT) Subject: Re: [PATCH] support: Add support_blob_repeat_allocate_shared To: Florian Weimer , libc-alpha@sourceware.org References: <87ftce1o6w.fsf@mid.deneb.enyo.de> From: Carlos O'Donell Organization: Red Hat Message-ID: <8b916fd0-ae97-e1e2-a6c7-1eb3af97161c@redhat.com> Date: Mon, 11 May 2020 23:39:52 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <87ftce1o6w.fsf@mid.deneb.enyo.de> Content-Language: en-US X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-13.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org 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: , X-List-Received-Date: Tue, 12 May 2020 03:40:01 -0000 On 5/5/20 9:55 AM, Florian Weimer wrote: > ----- Should be a 3-dash line. > support/blob_repeat.c | 31 +++++++++++++++++++++------ > support/blob_repeat.h | 12 ++++++++++- > support/tst-support_blob_repeat.c | 45 ++++++++++++++++++++++++++++----------- > 3 files changed, 68 insertions(+), 20 deletions(-) > No regressions on x86_64 and i686. OK for master. Reviewed-by: Carlos O'Donell. > diff --git a/support/blob_repeat.c b/support/blob_repeat.c > index a7aa9bf4c7..cd6297e026 100644 > --- a/support/blob_repeat.c > +++ b/support/blob_repeat.c > @@ -125,10 +125,11 @@ minimum_stride_size (size_t page_size, size_t element_size) > } > > /* Allocations larger than maximum_small_size potentially use mmap > - with alias mappings. */ > + with alias mappings. If SHARED, the alias mappings are created > + using MAP_SHARED instead of MAP_PRIVATE. */ OK. > static struct support_blob_repeat > allocate_big (size_t total_size, const void *element, size_t element_size, > - size_t count) > + size_t count, bool shared) OK. > { > unsigned long page_size = xsysconf (_SC_PAGESIZE); > size_t stride_size = minimum_stride_size (page_size, element_size); > @@ -213,7 +214,11 @@ allocate_big (size_t total_size, const void *element, size_t element_size, > { > size_t remaining_size = total_size; > char *current = target; > - int flags = MAP_FIXED | MAP_FILE | MAP_PRIVATE; > + int flags = MAP_FIXED | MAP_FILE; > + if (shared) > + flags |= MAP_SHARED; > + else > + flags |= MAP_PRIVATE; OK. > #ifdef MAP_NORESERVE > flags |= MAP_NORESERVE; > #endif > @@ -251,8 +256,8 @@ allocate_big (size_t total_size, const void *element, size_t element_size, > } > > struct support_blob_repeat > -support_blob_repeat_allocate (const void *element, size_t element_size, > - size_t count) > +repeat_allocate (const void *element, size_t element_size, > + size_t count, bool shared) OK. > { > size_t total_size; > if (__builtin_mul_overflow (element_size, count, &total_size)) > @@ -263,7 +268,21 @@ support_blob_repeat_allocate (const void *element, size_t element_size, > if (total_size <= maximum_small_size) > return allocate_malloc (total_size, element, element_size, count); > else > - return allocate_big (total_size, element, element_size, count); > + return allocate_big (total_size, element, element_size, count, shared); OK. Pass shared. > +} > + > +struct support_blob_repeat > +support_blob_repeat_allocate (const void *element, size_t element_size, > + size_t count) > +{ > + return repeat_allocate (element, element_size, count, false); OK. Non-shared. > +} > + > +struct support_blob_repeat > +support_blob_repeat_allocate_shared (const void *element, size_t element_size, > + size_t count) > +{ > + return repeat_allocate (element, element_size, count, true); OK. Shared. > } > > void > diff --git a/support/blob_repeat.h b/support/blob_repeat.h > index 12f33bcd02..519458cf50 100644 > --- a/support/blob_repeat.h > +++ b/support/blob_repeat.h > @@ -38,7 +38,17 @@ struct support_blob_repeat support_blob_repeat_allocate (const void *element, > size_t element_size, > size_t count); > > -/* Deallocate the blob created by support_blob_repeat_allocate. */ > +/* Like support_blob_repeat_allocate, except that copy-on-write > + semantics are disabled. This means writing to one part of the blob > + can affect other parts. It is possible to map non-shared memory > + over parts of the resulting blob using MAP_ANONYMOUS | MAP_FIXED > + | MAP_PRIVATE, so that writes to these parts do not affect > + others. */ > +struct support_blob_repeat support_blob_repeat_allocate_shared > + (const void *element, size_t element_size, size_t count); OK. > + > +/* Deallocate the blob created by support_blob_repeat_allocate or > + support_blob_repeat_allocate_shared. */ > void support_blob_repeat_free (struct support_blob_repeat *); > > #endif /* SUPPORT_BLOB_REPEAT_H */ > diff --git a/support/tst-support_blob_repeat.c b/support/tst-support_blob_repeat.c > index a0eb9d2b89..b61d6b249a 100644 > --- a/support/tst-support_blob_repeat.c > +++ b/support/tst-support_blob_repeat.c > @@ -17,6 +17,7 @@ > . */ > > #include > +#include OK. > #include > #include > > @@ -63,21 +64,39 @@ do_test (void) > } > support_blob_repeat_free (&repeat); > > - repeat = support_blob_repeat_allocate ("012345678", 9, 10 * 1000 * 1000); > - if (repeat.start == NULL) > - puts ("warning: not enough memory for large mapping"); > - else > + for (int do_shared = 0; do_shared < 2; ++do_shared) > { > - unsigned char *p = repeat.start; > - for (int i = 0; i < 10 * 1000 * 1000; ++i) > - for (int j = 0; j <= 8; ++j) > - if (p[i * 9 + j] != '0' + j) > - { > - printf ("error: element %d index %d\n", i, j); > - TEST_COMPARE (p[i * 9 + j], '0' + j); > - } > + if (do_shared) > + repeat = support_blob_repeat_allocate_shared ("012345678", 9, > + 10 * 1000 * 1000); OK. Do the shared and non-shared variants for testing. > + else > + repeat = support_blob_repeat_allocate ("012345678", 9, > + 10 * 1000 * 1000); > + if (repeat.start == NULL) > + puts ("warning: not enough memory for large mapping"); > + else > + { > + unsigned char *p = repeat.start; > + for (int i = 0; i < 10 * 1000 * 1000; ++i) > + for (int j = 0; j <= 8; ++j) > + if (p[i * 9 + j] != '0' + j) > + { > + printf ("error: element %d index %d\n", i, j); > + TEST_COMPARE (p[i * 9 + j], '0' + j); > + } > + > + enum { total_size = 9 * 10 * 1000 * 1000 }; > + p[total_size - 1] = '\0'; > + asm ("" ::: "memory"); > + if (do_shared) > + /* The write is repeated in multiple places earlier in the > + string due to page sharing. */ > + TEST_VERIFY (strlen (repeat.start) < total_size - 1); > + else > + TEST_COMPARE (strlen (repeat.start), total_size - 1); > + } > + support_blob_repeat_free (&repeat); OK. > } > - support_blob_repeat_free (&repeat); > > return 0; > } > -- Cheers, Carlos.