RE: [RFC][PATCH 09/10] locking/refcount: Provide refcount_dec_unless()

From: "Reshetova, Elena" <elena.reshetova@intel.com>
To: Peter Zijlstra <peterz@infradead.org>,
	Al Viro <viro@ZenIV.linux.org.uk>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Chris Mason <clm@fb.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"David Howells" <dhowells@redhat.com>,
	"ishkamiel@gmail.com" <ishkamiel@gmail.com>,
	"dwindsor@gmail.com" <dwindsor@gmail.com>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>
Subject: RE: [RFC][PATCH 09/10] locking/refcount: Provide refcount_dec_unless()
Date: Mon, 27 Feb 2017 09:28:39 +0000	[thread overview]
Message-ID: <2236FBA76BA1254E88B949DDB74E612B41C505C4@IRSMSX102.ger.corp.intel.com> (raw)
In-Reply-To: <20170224162044.479190330@infradead.org>

> By allowing a different unless value than 1, we can do dec_and_lock
> like things on higher values, like 2, which is useful if the data
> structure we lock also owns a reference (because in that case we'd
> never hit 1).
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  include/linux/refcount.h |    8 +++++++-
>  lib/refcount.c           |   14 ++++++++------
>  2 files changed, 15 insertions(+), 7 deletions(-)

Even if we decide not to touch inode->i_count because people believe it doesn't make that much sense, 
this API extension would be highly useful in other places. The consequences if an usage counter overflows can be also very much undesirable and I don't think the code that uses them is prepared to handle such cases. 
So we also need a way to address it if we want to get rid of problem in general.  
In some cases you can do a general +1 on counting scheme, benefit from an existing lock and convert them to refcount_t, but sometimes it is really not appropriate to take a new generic lock.

So, there are basically two ways for converting these cases:

- create a new type, usagecounter_t (put whatever name you like) (as we discussed before a number of times) and provide below functions there
- extend refcount_t type to have the below functions 

First approach might be overall cleaner, but extend even further the set of atomic-like primitives we already have. Some people get confused when given too many choices and default to atomic_t as "old, default option". We don't want this to happen.
Second approach is more compact, but its usage needs to be documented more precisely. Personally it is hard for me to think of a case when adding refcount_dec_unless() to API
would result in people misusing refcount_t API more than they would do without it. But maybe I am not creative enough? 

Best Regards,
Elena. 