From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753434AbcKUHsc (ORCPT ); Mon, 21 Nov 2016 02:48:32 -0500 Received: from mail-wm0-f65.google.com ([74.125.82.65]:36551 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752075AbcKUHsa (ORCPT ); Mon, 21 Nov 2016 02:48:30 -0500 Date: Mon, 21 Nov 2016 08:48:26 +0100 From: Ingo Molnar To: Boqun Feng 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: <20161121074826.GA29412@gmail.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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161121040644.GE5227@tardis.cn.ibm.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Boqun Feng wrote: > > It also fails to decrement in the underflow case (which is fine, but not > > obvious from the comment). Same thing below. > > > > Maybe a table in the comment like the following helps? > > /* > * 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 | > */ Yes! nit: s/fasle/false Also, I think we want to do a couple of other changes as well to make it more readable, extend the columns with 'normal' values (2 and UINT_MAX-2) and order the colums properly. I.e. something like: /* * 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'. */ I think this table makes the overflow/underflow semantics pretty clear and also documents the regular behavior of these atomic ops pretty intuitively. Agreed? Thanks, Ingo