From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752637AbcKUIiW (ORCPT ); Mon, 21 Nov 2016 03:38:22 -0500 Received: from mail-pg0-f66.google.com ([74.125.83.66]:35174 "EHLO mail-pg0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751428AbcKUIiU (ORCPT ); Mon, 21 Nov 2016 03:38:20 -0500 Date: Mon, 21 Nov 2016 16:38:22 +0800 From: Boqun Feng To: Ingo Molnar Cc: Will Deacon , Peter Zijlstra , "Reshetova, Elena" , "gregkh@linuxfoundation.org" , "keescook@chromium.org" , "arnd@arndb.de" , "tglx@linutronix.de" , "hpa@zytor.com" , "dave@progbits.org" , "linux-kernel@vger.kernel.org" Subject: Re: [RFC][PATCH 7/7] kref: Implement using refcount_t Message-ID: <20161121083822.GF5227@tardis.cn.ibm.com> References: <20161114173946.501528675@infradead.org> <20161114174446.832175072@infradead.org> <2236FBA76BA1254E88B949DDB74E612B41C148C4@IRSMSX102.ger.corp.intel.com> <20161118113718.GP3117@twins.programming.kicks-ass.net> <20161118170655.GZ13470@arm.com> <20161121040644.GE5227@tardis.cn.ibm.com> <20161121074826.GA29412@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="nqkreNcslJAfgyzk" Content-Disposition: inline In-Reply-To: <20161121074826.GA29412@gmail.com> User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --nqkreNcslJAfgyzk Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Nov 21, 2016 at 08:48:26AM +0100, Ingo Molnar wrote: >=20 > * Boqun Feng wrote: >=20 > > > It also fails to decrement in the underflow case (which is fine, but = not > > > obvious from the comment). Same thing below. > > >=20 > >=20 > > Maybe a table in the comment like the following helps? > >=20 > > /* > > * T: return true, F: return fasle > > * W: trigger WARNING > > * N: no effect > > * > > * | value before ops | > > * | 0 | 1 | UINT_MAX - 1 | UINT_MAX | > > * ---------------------+-------+-------+--------------+----------+ > > * inc() | W | | W | N | > > * inc_not_zero() | FN | T | WT | WTN | > > * dec_and_test() | WFN | T | F | FN | > > * dec_and_mutex_lock() | WFN | T | F | FN | > > * dec_and_spin_lock() | WFN | T | F | FN | > > */ >=20 > Yes! >=20 > nit: s/fasle/false >=20 > Also, I think we want to do a couple of other changes as well to make it = more=20 > readable, extend the columns with 'normal' values (2 and UINT_MAX-2) and = order the=20 > colums properly. I.e. something like: >=20 > /* > * The before/after outcome of various atomic ops: > * > * T: returns true > * F: returns false > * ---------------------------------- > * W: op triggers kernel WARNING > * ---------------------------------- > * 0: no change to atomic var value > * +: atomic var value increases by 1 > * -: atomic var value decreases by 1 > * ---------------------------------- > * -1: UINT_MAX > * -2: UINT_MAX-1 > * -3: UINT_MAX-2 > * > * ---------------------+-----+-----+-----+-----+-----+-----+ > * value before: | -3 | -2 | -1 | 0 | 1 | 2 | > * ---------------------+-----+-----+-----+-----+-----+-----+ > * value+effect after: | > * ---------------------+ | | | | | | > * inc() | ..+ | W.+ | ..0 | W.+ | ..+ | ..+ | > * inc_not_zero() | .T+ | WT+ | WT0 | .F0 | .T+ | .T+ | > * dec_and_test() | .F- | .F- | .F0 | WF0 | .T- | .F- | > * dec_and_mutex_lock() | .F- | .F- | .F0 | WF0 | .T- | .F- | > * dec_and_spin_lock() | .F- | .F- | .F0 | WF0 | .T- | .F- | > * ---------------------+-----+-----+-----+-----+-----+-----+ > * > * So for example: 'WT+' in the inc_not_zero() row and '-2' column > * means that when the atomic_inc_not_zero() function is called > * with an atomic var that has a value of UINT_MAX-1, then the > * atomic var's value will increase to the maximum overflow value > * of UINT_MAX and will produce a warning. The function returns > * 'true'. > */ >=20 > I think this table makes the overflow/underflow semantics pretty clear an= d also=20 > documents the regular behavior of these atomic ops pretty intuitively. >=20 > Agreed? >=20 Sure, this looks pretty great! Much more informative and readable than my version ;-) Thank you. Regards, Boqun > Thanks, >=20 > Ingo --nqkreNcslJAfgyzk Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEcBAABCAAGBQJYMrJ6AAoJEEl56MO1B/q4mdAIAKWWiRoF3Xojq8X3o0t27Pwa taa5Qkgm/O/TWMWgBKXJARU/Mbd65KMszH+6Qu5dpk+strNJKgmOez2Tt3ErWPoI GzpqF+fRQoQXCDmcF+lRTQ46BeZ8kzxqFRRGqpn/JrsqLo4fV4+89Uqt+7tZUhu5 4dhol7/sFeQobf2NzVy8/cu8OfJ9WhhFY2SlOY++yfkX+EOAIOt9U3q7zKGGrkqg jHz99de1C/uTFHcUl4ulind7xp6G+2f2u07R+rjG/3vnKrMDp6ZzoRbZ5sWP+gfE lKNdOIxaeDjy1BAJhQj55mS8K+nJe916VzKRN3Jz/gTypmIWNJ7x4+cNEQ+th0A= =at99 -----END PGP SIGNATURE----- --nqkreNcslJAfgyzk--