From owner-svn-src-all@freebsd.org Tue Nov 17 14:36:34 2020 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 779C82EE7A1; Tue, 17 Nov 2020 14:36:34 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-wm1-x341.google.com (mail-wm1-x341.google.com [IPv6:2a00:1450:4864:20::341]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Cb7mf2c9Fz4kHS; Tue, 17 Nov 2020 14:36:34 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by mail-wm1-x341.google.com with SMTP id w24so3452412wmi.0; Tue, 17 Nov 2020 06:36:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=9GsqJsfYrii5z1WrNicZCaObgFeFUC9In8ZBB63VaVY=; b=mZLdAa/K0V5BRPIHeFcOflZpsNz0rxU4bQl3DsxcPySddWBPDCiZYRNWWCE8fhEJno Ms4UFDH3jAIamKnamXxeSaCPtc/gbq5sS/sffdjPu2rZgOMiUcikFk0JnYgeKx7zgjwX f/VIPV1GNqrhTHGnguSN1tzf4vYeQZxFesm+IaRjjgrBb9tdxmH9reMwhNyEguvrWMH1 pZONH49IUbzu3UpQ5UI637jEDcXfnVN7swCSLOTi/CEqZsgy9voGO5IHoOh8AjOIQRJN +8nM/LoOYdbDJ+J6ztGOaGaS9OEZ1TLPMiXFMrozMxXP5a5vyAJEWt8iVtJWDsNX9zIn +3Gg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=9GsqJsfYrii5z1WrNicZCaObgFeFUC9In8ZBB63VaVY=; b=p7V+lQhJ2Jtrtm4qV2tcq4xFKMev1nYvXAZVnv155C0TgxGS7Gsc3e7WDyjOWewa6F S8bKy56u97Efgi5EpFPm7OjzPVQVl1Wv71f3twmHzYkfYOBSOVLNdfAORj/Dqa/3Sc7O sLi9wXehFODkMkiNfrqW0lFPztM0PMf+cAvR7uVeDUQ2DFYxDlIFc0dg0JNkqcTST/mA avWLn14QTqL7Geb+81RXiZbKjhSko87HOn9QEYJkkvkQkINqcsJQ68LJV9q+J5lEl4no RaAxU3b+3U8wLlIQu+/qOO6ftL0BV1reOFdOO5JVn9kCdRxeE/6/CQ1Yd5Kkg21t1Bf4 vJVg== X-Gm-Message-State: AOAM5324uzSKUu4tBVR3sfIyou+kgpiXaIPlbCH4VOpvBwaLnWmGqfMI 8oQGuV6OfEPdqsGMjmapPnSpIdVjKoRFLxZ4Q3U= X-Google-Smtp-Source: ABdhPJwM0VdSOMBM7wggA9yLVXMUszi3G6JVOuYBm7tzwMhjAMzjVrVPdxQ7YKV06fI2pqOStLi6G6b2Kev5jpHMN3M= X-Received: by 2002:a7b:cd11:: with SMTP id f17mr4827516wmj.127.1605623792666; Tue, 17 Nov 2020 06:36:32 -0800 (PST) MIME-Version: 1.0 Received: by 2002:adf:dec7:0:0:0:0:0 with HTTP; Tue, 17 Nov 2020 06:36:31 -0800 (PST) In-Reply-To: References: <202011160309.0AG39JmP067464@repo.freebsd.org> From: Mateusz Guzik Date: Tue, 17 Nov 2020 15:36:31 +0100 Message-ID: Subject: Re: svn commit: r367713 - head/sys/kern To: Konstantin Belousov Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 4Cb7mf2c9Fz4kHS X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 Nov 2020 14:36:34 -0000 On 11/17/20, Konstantin Belousov wrote: > On Tue, Nov 17, 2020 at 04:15:12AM +0100, Mateusz Guzik wrote: >> On 11/17/20, Konstantin Belousov wrote: >> > On Mon, Nov 16, 2020 at 03:09:19AM +0000, Mateusz Guzik wrote: >> >> Author: mjg >> >> Date: Mon Nov 16 03:09:18 2020 >> >> New Revision: 367713 >> >> URL: https://svnweb.freebsd.org/changeset/base/367713 >> >> >> >> Log: >> >> select: replace reference counting with memory barriers in selfd >> >> >> >> Refcounting was added to combat a race between selfdfree and >> >> doselwakup, >> >> but it adds avoidable overhead. >> >> >> >> selfdfree detects it can free the object by ->sf_si == NULL, thus we >> >> can >> >> ensure that the condition only holds after all accesses are >> >> completed. >> >> >> >> Modified: >> >> head/sys/kern/sys_generic.c >> >> >> >> Modified: head/sys/kern/sys_generic.c >> >> ============================================================================== >> >> --- head/sys/kern/sys_generic.c Sun Nov 15 22:49:28 2020 (r367712) >> >> +++ head/sys/kern/sys_generic.c Mon Nov 16 03:09:18 2020 (r367713) >> >> @@ -156,7 +156,6 @@ struct selfd { >> >> struct mtx *sf_mtx; /* Pointer to selinfo mtx. */ >> >> struct seltd *sf_td; /* (k) owning seltd. */ >> >> void *sf_cookie; /* (k) fd or pollfd. */ >> >> - u_int sf_refs; >> >> }; >> >> >> >> MALLOC_DEFINE(M_SELFD, "selfd", "selfd"); >> >> @@ -1704,16 +1703,17 @@ static void >> >> selfdfree(struct seltd *stp, struct selfd *sfp) >> >> { >> >> STAILQ_REMOVE(&stp->st_selq, sfp, selfd, sf_link); >> >> - if (sfp->sf_si != NULL) { >> >> + /* >> >> + * Paired with doselwakeup. >> >> + */ >> >> + if (atomic_load_acq_ptr((uintptr_t *)&sfp->sf_si) != (uintptr_t)NULL) >> >> { >> > This could be != 0. >> > >> >> mtx_lock(sfp->sf_mtx); >> >> if (sfp->sf_si != NULL) { >> >> TAILQ_REMOVE(&sfp->sf_si->si_tdlist, sfp, sf_threads); >> >> - refcount_release(&sfp->sf_refs); >> >> } >> >> mtx_unlock(sfp->sf_mtx); >> >> } >> >> - if (refcount_release(&sfp->sf_refs)) >> >> - free(sfp, M_SELFD); >> >> + free(sfp, M_SELFD); >> > What guarantees that doselwakeup() finished with sfp ? >> > >> >> Release semantics provided by atomic_store_rel_ptr -- it means the >> NULL store is the last access, neither CPU nor the compiler are going >> to reorder preceding loads/stores past it. > It only guarantees that if we observed NULL as the result of load. > If that did not happen selfdfree takes the lock. If the entry is still there, it removes it and doselwakeup will not see it after it takes the lock. If the entry is not there, doselwaekup is done with it because it released the lock. -- Mateusz Guzik