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=-5.5 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI,NICE_REPLY_A, 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 DF1781F8C6 for ; Thu, 26 Aug 2021 12:14:52 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id DB9513858434 for ; Thu, 26 Aug 2021 12:14:51 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org DB9513858434 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1629980091; bh=wqFDxb5lIpY+SsS6DzEf4jxp0Mz2mB7VOHrPmS0q03I=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=vtGAomZ3r4oC84rdvm/nVAVggaXpaqExSniE/yWNyJRgL6VJQWLG30G4VXRnjs3AO ce2bj/Zx8k4k38E/nSCgMNOTcC37zjg8mxv9UFYS3lARlceW0r3o8AxlUNDxPIZ+XH sxwKhU3pw+D1SP299GSLsl7uhM4a/OpM9fOMFtQk= Received: from mail-qk1-x72e.google.com (mail-qk1-x72e.google.com [IPv6:2607:f8b0:4864:20::72e]) by sourceware.org (Postfix) with ESMTPS id 964053858402 for ; Thu, 26 Aug 2021 12:14:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 964053858402 Received: by mail-qk1-x72e.google.com with SMTP id m21so2962146qkm.13 for ; Thu, 26 Aug 2021 05:14:22 -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:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=wqFDxb5lIpY+SsS6DzEf4jxp0Mz2mB7VOHrPmS0q03I=; b=emt6HBE62u+tO1qhjgx1fGeI09mSXI+r6ZbbcHq2wB39UlUuGL0T8Kp7nABg0DkS3C mC3F7026xD799Q/H1ups01j3vrpstSie4QVTHEs3H6XJu1uMBbWvGYkgk8qNaIgJKcvJ BWGe+PfYtBb+6v0GaXq5oJ+QVAybrT4tMTlXmV7K0bpKkF3w2Nj6QqMglO4zCmkMzgol dIt6LBKx0BalspRRL+yZCjAkohj5sQNdLSPB2S/sfGeWlQrZASi+P2pQShp3C4WJoG+B HMBRdCiaGSVd12onzx0FWQONSNHo5U3tje58lrwmuERYXEo+qAeomhj1bpqtLpzOCn/m RxiA== X-Gm-Message-State: AOAM531Bd5aT91rglwA2OsxrkOl7Mrb3HuxrzVm6znuYby2t0NhGjoPF nU2XXdHDAKoERh8zwocAFKcAcapcmsWQlg== X-Google-Smtp-Source: ABdhPJzam1ZKsoOUHA/DwEgbzWiaegKJbwpk8oz52TpzqTESamy7sJMV9m0PaKAonX6uCb1LJLxKjA== X-Received: by 2002:a37:9cc8:: with SMTP id f191mr3253358qke.113.1629980061997; Thu, 26 Aug 2021 05:14:21 -0700 (PDT) Received: from ?IPv6:2804:431:c7ca:1a68:4988:5e6e:21a3:3c6b? ([2804:431:c7ca:1a68:4988:5e6e:21a3:3c6b]) by smtp.gmail.com with ESMTPSA id 75sm2306351qko.100.2021.08.26.05.14.21 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 26 Aug 2021 05:14:21 -0700 (PDT) Subject: Re: [PATCH v2 03/19] nptl: Handle robust PI mutexes for !__ASSUME_SET_ROBUST_LIST To: Florian Weimer References: <20210823195047.543237-1-adhemerval.zanella@linaro.org> <20210823195047.543237-4-adhemerval.zanella@linaro.org> <87lf4o8rux.fsf@oldenburg.str.redhat.com> Message-ID: <6f7d9204-6a81-a021-30b4-e3bbea580fae@linaro.org> Date: Thu, 26 Aug 2021 09:14:19 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <87lf4o8rux.fsf@oldenburg.str.redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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: , From: Adhemerval Zanella via Libc-alpha Reply-To: Adhemerval Zanella Cc: libc-alpha@sourceware.org Errors-To: libc-alpha-bounces+e=80x24.org@sourceware.org Sender: "Libc-alpha" On 26/08/2021 06:42, Florian Weimer wrote: > * Adhemerval Zanella: > >> The robust PI mutexes are signaled by setting the LSB bit to 1, so >> the code requires to take this consideration before access the >> __pthread_mutex_s. >> >> The code is also simplified: the initialization code is not really >> required, PD->robust_head.list and PD->robust_list.__next are >> essentially the same regardless of __PTHREAD_MUTEX_HAVE_PREV, the futex >> wake is optimized to be issued only when required, and the futex shared >> bit is set only when required. > > Is this a user-visible bug? Should it have a bug reference? This a bug on only visible on a handful of architectures where __NR_set_robust_list is not support due the missing CONFIG_HAVE_FUTEX_CMPXCHG. I have opened https://sourceware.org/bugzilla/show_bug.cgi?id=28268 > >> diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c >> index d8ec299cb1..08e5189ad6 100644 >> --- a/nptl/pthread_create.c >> +++ b/nptl/pthread_create.c >> @@ -486,35 +486,36 @@ start_thread (void *arg) >> exit (0); >> >> #ifndef __ASSUME_SET_ROBUST_LIST >> - /* If this thread has any robust mutexes locked, handle them now. */ >> -# if __PTHREAD_MUTEX_HAVE_PREV >> - void *robust = pd->robust_head.list; >> -# else >> - __pthread_slist_t *robust = pd->robust_list.__next; >> -# endif >> - /* We let the kernel do the notification if it is able to do so. >> - If we have to do it here there for sure are no PI mutexes involved >> - since the kernel support for them is even more recent. */ >> - if (!__nptl_set_robust_list_avail >> - && __builtin_expect (robust != (void *) &pd->robust_head, 0)) >> + /* We let the kernel do the notification if it is able to do so on the exit >> + syscall. Otherwise we need to handle before the thread terminates. */ >> + void **robust; >> + while ((robust = pd->robust_head.list) >> + && robust != (void *) &pd->robust_head) >> { >> - do >> + /* Note: robust PI futexes are signaled by setting bit 0. */ >> + void **robustp = (void **) ((uintptr_t) robust & ~1UL); >> + >> + struct __pthread_mutex_s *mtx = (struct __pthread_mutex_s *) >> + ((char *) robustp - offsetof (struct __pthread_mutex_s, >> + __list.__next)); >> + unsigned int nusers = mtx->__nusers; >> + int shared = mtx->__kind & 128; >> + >> + pd->robust_head.list_op_pending = robust; >> + pd->robust_head.list = *robustp; >> + /* Although the list will not be changed at this point, it follows the >> + expected kernel ABI. */ >> + __asm ("" ::: "memory"); >> + >> + int lock = atomic_exchange_relaxed (&mtx->__lock, FUTEX_OWNER_DIED); >> + /* Wake any users if mutex is acquired with potential users. */ >> + if (lock > 1 || nusers != 0) > > Why the check for nusers? Isn't that racy? I was small optimization to avoid calling the futex, but I think you are correct that is racy. I think it would be better to let the kernel handle it. > > Thanks, > Florian >