From owner-svn-src-head@freebsd.org Sat Oct 14 15:16:10 2017 Return-Path: Delivered-To: svn-src-head@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 70FD7E4973C; Sat, 14 Oct 2017 15:16:10 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-qt0-x22d.google.com (mail-qt0-x22d.google.com [IPv6:2607:f8b0:400d:c0d::22d]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 20F3F72F3A; Sat, 14 Oct 2017 15:16:10 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by mail-qt0-x22d.google.com with SMTP id f15so24502130qtf.7; Sat, 14 Oct 2017 08:16:10 -0700 (PDT) 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=opTdpQUfje3NYh95R37aKLvadG2jBp5lyBKb3Fi/Vq4=; b=Nny+qOxlfNYalM2n4BRD1REAxkGZ0H2o7XW/xMPaXq8lHTpUUuzIIT8MyJVKDKceeo O/jCIO7jUpQ0k9aWpq/qdzo8VL8mcN9Z7N2nLHE+EMcaRwJVjzPXT7gGbAdZT5C2rbXg mfG/t6iLPu38XhXvQckpSXxW8OtK7zJhy0b49psEQIfSyey/rUc8KOnC8IFFz1uQ6Eac xDzYwIYGkLzTCt7Pq5G6kjp205h3+ocCEHHalKce+r6vNloGrJbXFfjZm63MGGhPEQ65 WdrEkWtc/97ZT83iNh4GGWmDbog6K8vieJN+nNmOYTSOvKwaBUgSOIrKsJCgIQWtjQdj zn2g== 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=opTdpQUfje3NYh95R37aKLvadG2jBp5lyBKb3Fi/Vq4=; b=nmvlt2bnqCLY2F/SDiObP9F02i0icfZWOkXYGH4fu7aIFCCRzZOxEkmfDymNTLKP1l Q9tYiLNzWEqcEPSCzz37dGBRWsYaIEBIYjWePq3WjRP17xKZRvUGcyvFtnyphPQsUNF7 o0WNLsx/9PTj0ZovO+vVc2by19QvkA5HXt1LKTR6hxpqYwiS34UHM9VeptnykCSmYKqt s4ONpzT4TOxKuz5wG7UgbZec25WJeRsTYVzweoPUwsh7HDmxqfjWX/EhKCEz1DtgHZuf 0mZto2Hix2mH7XXL8vJ6tU+pXR2rK5pdhCm2lxahxjN0Nj5JobWxqJ1gKNPtmEMRdpas /xdw== X-Gm-Message-State: AMCzsaVg+WMqVC/4TYdvR6C8smIYvG/u8jTp7RHaQdx9YPWhVLVOw5bw 9gXtewYFVp8D0xU44qb7owHS++nzf4j5M6tWFy7WMQ== X-Google-Smtp-Source: ABhQp+T+naHyeVTQxCd+En91UkuMYg9h8zTXLCFG6STVwpc/Ql5mHWrkfYDe0hikvvSNY9VVnx5sQAp+dIEoA0yYM2U= X-Received: by 10.237.63.202 with SMTP id w10mr2191990qth.56.1507994169185; Sat, 14 Oct 2017 08:16:09 -0700 (PDT) MIME-Version: 1.0 Received: by 10.237.51.167 with HTTP; Sat, 14 Oct 2017 08:16:08 -0700 (PDT) In-Reply-To: References: <201710132031.v9DKVueS089009@repo.freebsd.org> <1507941050.8386.88.camel@freebsd.org> From: Mateusz Guzik Date: Sat, 14 Oct 2017 17:16:08 +0200 Message-ID: Subject: Re: svn commit: r324609 - head/sys/sys To: Svatopluk Kraus Cc: Ian Lepore , Mateusz Guzik , "src-committers@freebsd.org" , "svn-src-all@freebsd.org" , "svn-src-head@freebsd.org" Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.23 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 14 Oct 2017 15:16:10 -0000 On Sat, Oct 14, 2017 at 11:23 AM, Svatopluk Kraus wrote: > On Sat, Oct 14, 2017 at 2:48 AM, Mateusz Guzik wrote: > > Justification for the change was provided in the commit message: it makes > > .text smaller on amd64 and probably all architectures. > > > > I did not lack an explanation why you did it, but why it's possible > and safe. Something like this: > ----------------- > (1) MTX_UNOWNED even if defined like a flag was never used like a > flag. It was used like a value set for unowned mutex and test > accordingly. There is no logical operation (AND, OR, ...) done with > MTX_UNOWNED in code. There are only assignments and checks for > equality there. Thus, we can change its value and do not pretend that > it's a flag anymore. As mutex owner thread pointer and mutex flags are > kept in one variable together, setting MTX_UNOWNED value to 0 is more > appropriate and brings some benefits. However, MTX_DESTROYED flag must > be distinguish from MTX_CONTESTED one now, so there is no savings > considering mutex flags. > (2) MTX_UNONWED value is used only internally within mutex > implemention, thus this change should be quite safe. > ----------------- > I guess that's a fair point, I figured some people may be wondering why switching to 0 makes any difference. On the other hand anyone worried about var usage can easily grep. Next time I'll be more verbose. > > However, I suggest to do something with MTX_UNOWNED leak in > sys/dev/syscons/syscons.c. i.e. to make mtx_unowned() public and use > it instead. Further, MTX_DESTROYED is not used like a flag too. So, > was its redefinition really needed? In other words, isn't > MTX_CONTESTED flag without owner thread pointer set enough? Also, I > would prefer to have some explanation to be in mutex.h about all of > this. At least to not mess flags with values in definitions without > explanation. > > The code in syscons looks extremely fishy, I don't know what to do with it yet. Testing for ownership should be avoidable with trylock. What you are proposing is an equivalent to the previous trick, but requires reading the lock owner. All places testing for MTX_DESTROYED would have to be updated which is an avoidable churn right now. Not employing the hack makes the flags self-explanatory. If a need for a new flag arises there are still 2 free bits and if that's not enough we can alter the current code. -- Mateusz Guzik