linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] refcount: introduce refcount_is_one() helper function
@ 2021-11-18  3:53 Yajun Deng
  2021-11-18  7:42 ` Peter Zijlstra
  2021-11-18  8:12 ` yajun.deng
  0 siblings, 2 replies; 6+ messages in thread
From: Yajun Deng @ 2021-11-18  3:53 UTC (permalink / raw)
  To: will, peterz, boqun.feng; +Cc: linux-kernel, Yajun Deng

There are many cases where it is necessary to determine if refcount is one,
introduce refcount_is_one() helper function for these cases.

Signed-off-by: Yajun Deng <yajun.deng@linux.dev>
---
 include/linux/refcount.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/refcount.h b/include/linux/refcount.h
index b8a6e387f8f9..1cc23c0e7454 100644
--- a/include/linux/refcount.h
+++ b/include/linux/refcount.h
@@ -147,6 +147,11 @@ static inline unsigned int refcount_read(const refcount_t *r)
 	return atomic_read(&r->refs);
 }
 
+static inline bool refcount_is_one(const refcount_t *r)
+{
+	return refcount_read(r) == 1;
+}
+
 static inline __must_check bool __refcount_add_not_zero(int i, refcount_t *r, int *oldp)
 {
 	int old = refcount_read(r);
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] refcount: introduce refcount_is_one() helper function
  2021-11-18  3:53 [PATCH] refcount: introduce refcount_is_one() helper function Yajun Deng
@ 2021-11-18  7:42 ` Peter Zijlstra
  2021-11-18  8:12 ` yajun.deng
  1 sibling, 0 replies; 6+ messages in thread
From: Peter Zijlstra @ 2021-11-18  7:42 UTC (permalink / raw)
  To: Yajun Deng; +Cc: will, boqun.feng, linux-kernel

On Thu, Nov 18, 2021 at 11:53:28AM +0800, Yajun Deng wrote:
> There are many cases where it is necessary to determine if refcount is one,
> introduce refcount_is_one() helper function for these cases.

Give me one that is not racy?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] refcount: introduce refcount_is_one() helper function
  2021-11-18  3:53 [PATCH] refcount: introduce refcount_is_one() helper function Yajun Deng
  2021-11-18  7:42 ` Peter Zijlstra
@ 2021-11-18  8:12 ` yajun.deng
  2021-11-18  8:44   ` Peter Zijlstra
  2021-11-18 10:34   ` yajun.deng
  1 sibling, 2 replies; 6+ messages in thread
From: yajun.deng @ 2021-11-18  8:12 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: will, boqun.feng, linux-kernel

November 18, 2021 3:42 PM, "Peter Zijlstra" <peterz@infradead.org> wrote:

> On Thu, Nov 18, 2021 at 11:53:28AM +0800, Yajun Deng wrote:
> 
>> There are many cases where it is necessary to determine if refcount is one,
>> introduce refcount_is_one() helper function for these cases.
> 
> Give me one that is not racy?

The following determine refcount is one, 35 count.
linux (next-20211117) $ grep "refcount_read.*== 1"  ./  -RnI
./tools/lib/perf/mmap.c:              101:	    if (refcount_read(&map->refcnt) == 1 && perf_mmap__empty(map))
./tools/perf/tests/cpumap.c:           72:      TEST_ASSERT_VAL("wrong refcnt", refcount_read(&map->refcnt) == 1);
./tools/perf/tests/thread-map.c:       42:		refcount_read(&map->refcnt) == 1);
./tools/perf/tests/thread-map.c:       57:		refcount_read(&map->refcnt) == 1);
./tools/perf/tests/thread-map.c:       84:	    refcount_read(&threads->refcnt) == 1);
./tools/perf/tests/maps.c:             28:		TEST_ASSERT_VAL("wrong map refcnt", refcount_read(&map->refcnt) == 1);
./include/linux/skbuff.h:            1078:	    if (likely(refcount_read(&skb->users) == 1))
./include/net/sock.h:                 726:		WARN_ON(refcount_read(&sk->sk_refcnt) == 1);
./include/net/sock.h:                 748:		WARN_ON(refcount_read(&sk->sk_refcnt) == 1);
./net/core/neighbour.c:               214:	    if (refcount_read(&n->refcnt) == 1) {
./net/core/neighbour.c:               264:		if (refcount_read(&n->refcnt) == 1) {
./net/core/neighbour.c:               965:		if (refcount_read(&n->refcnt) == 1 &&
./net/core/dev.c:                    3017:	    if (likely(refcount_read(&skb->users) == 1)) {
./net/core/skbuff.c:                  710:		if (refcount_read(&fclones->fclone_ref) == 1)
./net/core/skbuff.c:                 1514:	    refcount_read(&fclones->fclone_ref) == 1) {
./net/core/skbuff.c:                 6278:	    if (refcount_read(&old->refcnt) == 1)
./net/core/skbuff.c:                 6402:		refcount_read(&ext->refcnt) == 1) {
./net/core/skbuff.c:                 6417:	    if (refcount_read(&ext->refcnt) == 1)
./net/netlink/af_netlink.c:           614:		WARN_ON(refcount_read(&sk->sk_refcnt) == 1);
./net/tipc/socket.c:                 3032:		WARN_ON(refcount_read(&sk->sk_refcnt) == 1);
./net/netfilter/ipvs/ip_vs_conn.c:    480:	    (refcount_read(&cp->refcnt) == 1) &&
./net/sctp/auth.c:998:	    refcount_read(&key->refcnt) == 1) {
./drivers/tty/serial/sb1250-duart.c:  698:	    if (refcount_read(&duart->map_guard) == 1) {
./drivers/infiniband/core/cm.c:      1048:	    WARN_ON(refcount_read(&cm_id_priv->refcount) == 1);
./drivers/infiniband/hw/irdma/utils.c:513:		refcount_read(&cqp_request->refcnt) == 1, 1000);
./drivers/crypto/caam/qi.c:           306:		if (refcount_read(&drv_ctx->refcnt) == 1)
./drivers/net/ethernet/mellanox/mlx5/core/fs_core.c:1633:		if (refcount_read(&handle->rule[i]->node.refcount) == 1) {
./drivers/net/vxlan.c:               1537:	    if (family == AF_INET && sock4 && refcount_read(&sock4->refcnt) == 1)
./drivers/net/vxlan.c:               1541:	    if (family == AF_INET6 && sock6 && refcount_read(&sock6->refcnt) == 1)
./kernel/events/core.c:             12861:		wait_var_event(&ctx->refcount, refcount_read(&ctx->refcount) == 1);
./fs/nfs/nfs4proc.c:                 9127:		if (refcount_read(&clp->cl_count) == 1)
./fs/lockd/mon.c:                     195:	    if (refcount_read(&nsm->sm_count) == 1
./fs/btrfs/transaction.c:            2061:	    ASSERT(refcount_read(&trans->use_count) == 1);
./fs/btrfs/block-group.c:            3960:		ASSERT(refcount_read(&block_group->refs) == 1);
./fs/afs/write.c:                     921:	    if (refcount_read(&wbk->usage) == 1)

The following determine refcount isn't one, 19 count.
linux (next-20211117) $ grep "refcount_read.*!= 1"  ./  -RnI
./include/linux/skbuff.h:                                1762:	return refcount_read(&skb->users) != 1;
./include/net/sock.h:                                    1297:	if (refcount_read(&sk->sk_refcnt) != 1)
./crypto/algapi.c:                                        461:	BUG_ON(refcount_read(&alg->cra_refcnt) != 1);
./crypto/algapi.c:                                        560:	BUG_ON(refcount_read(&inst->alg.cra_refcnt) != 1);
./net/sunrpc/auth.c:                                      690:	if (refcount_read(&cred->cr_count) != 1 ||
./net/llc/llc_conn.c:                                     977:	if (refcount_read(&sk->sk_refcnt) != 1) {
./net/ceph/osd_client.c:                                 5297:	WARN_ON(refcount_read(&osdc->homeless_osd.o_ref) != 1);
./net/ipv6/ip6_fib.c:                                    1037:	if (refcount_read(&rt->fib6_ref) != 1) {
./net/core/neighbour.c:                                   349:	if (refcount_read(&n->refcnt) != 1) {
./net/core/pktgen.c:                                     3430:	while (refcount_read(&(pkt_dev->skb->users)) != 1) {
./drivers/crypto/marvell/octeontx/otx_cptvf_algs.c:      1584:	if (refcount_read(&otx_cpt_skciphers[i].base.cra_refcnt) != 1)
./drivers/crypto/marvell/octeontx/otx_cptvf_algs.c:      1587:	if (refcount_read(&otx_cpt_aeads[i].base.cra_refcnt) != 1)
./drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_atcam.c:406:		refcount_read(&lkey_id->refcnt) != 1, lkey_id->id,
./drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_atcam.c:435:			     refcount_read(&lkey_id->refcnt) != 1,
./drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_atcam.c:464:			     refcount_read(&lkey_id->refcnt) != 1, lkey_id->id,
./drivers/block/nbd.c:                                    2541:		if (refcount_read(&nbd->refs) != 1)
./fs/exec.c:                                              1182:	if (refcount_read(&oldsighand->count) != 1) {
./fs/btrfs/tests/extent-map-tests.c:                        24:		if (refcount_read(&em->refs) != 1) {
./arch/s390/mm/extmem.c:                                   472:	if (refcount_read(&seg->ref_count) != 1) {

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] refcount: introduce refcount_is_one() helper function
  2021-11-18  8:12 ` yajun.deng
@ 2021-11-18  8:44   ` Peter Zijlstra
  2021-11-18 10:34   ` yajun.deng
  1 sibling, 0 replies; 6+ messages in thread
From: Peter Zijlstra @ 2021-11-18  8:44 UTC (permalink / raw)
  To: yajun.deng; +Cc: will, boqun.feng, linux-kernel

On Thu, Nov 18, 2021 at 08:12:56AM +0000, yajun.deng@linux.dev wrote:
> November 18, 2021 3:42 PM, "Peter Zijlstra" <peterz@infradead.org> wrote:
> 
> > On Thu, Nov 18, 2021 at 11:53:28AM +0800, Yajun Deng wrote:
> > 
> >> There are many cases where it is necessary to determine if refcount is one,
> >> introduce refcount_is_one() helper function for these cases.
> > 
> > Give me one that is not racy?
> 
> The following determine refcount is one, 35 count.

Very good, now get me one that isn't broken :-)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] refcount: introduce refcount_is_one() helper function
  2021-11-18  8:12 ` yajun.deng
  2021-11-18  8:44   ` Peter Zijlstra
@ 2021-11-18 10:34   ` yajun.deng
  2021-11-18 10:49     ` Peter Zijlstra
  1 sibling, 1 reply; 6+ messages in thread
From: yajun.deng @ 2021-11-18 10:34 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: will, boqun.feng, linux-kernel

November 18, 2021 4:44 PM, "Peter Zijlstra" <peterz@infradead.org> wrote:

> On Thu, Nov 18, 2021 at 08:12:56AM +0000, yajun.deng@linux.dev wrote:
> 
>> November 18, 2021 3:42 PM, "Peter Zijlstra" <peterz@infradead.org> wrote:
>> 
>> On Thu, Nov 18, 2021 at 11:53:28AM +0800, Yajun Deng wrote:
>> 
>> There are many cases where it is necessary to determine if refcount is one,
>> introduce refcount_is_one() helper function for these cases.
>> 
>> Give me one that is not racy?
>> 
>> The following determine refcount is one, 35 count.
> 
> Very good, now get me one that isn't broken :-)

Sorry, I didn't understand what is the 'isn't broken'。

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] refcount: introduce refcount_is_one() helper function
  2021-11-18 10:34   ` yajun.deng
@ 2021-11-18 10:49     ` Peter Zijlstra
  0 siblings, 0 replies; 6+ messages in thread
From: Peter Zijlstra @ 2021-11-18 10:49 UTC (permalink / raw)
  To: yajun.deng; +Cc: will, boqun.feng, linux-kernel

On Thu, Nov 18, 2021 at 10:34:44AM +0000, yajun.deng@linux.dev wrote:
> November 18, 2021 4:44 PM, "Peter Zijlstra" <peterz@infradead.org> wrote:
> 
> > On Thu, Nov 18, 2021 at 08:12:56AM +0000, yajun.deng@linux.dev wrote:
> > 
> >> November 18, 2021 3:42 PM, "Peter Zijlstra" <peterz@infradead.org> wrote:
> >> 
> >> On Thu, Nov 18, 2021 at 11:53:28AM +0800, Yajun Deng wrote:
> >> 
> >> There are many cases where it is necessary to determine if refcount is one,
> >> introduce refcount_is_one() helper function for these cases.
> >> 
> >> Give me one that is not racy?
> >> 
> >> The following determine refcount is one, 35 count.
> > 
> > Very good, now get me one that isn't broken :-)
> 
> Sorry, I didn't understand what is the 'isn't broken'。

What's the value of refcount_read() given that at any moment a
concurrent refcount_{inc,dec}() can happen?

If you can't know the current value (per the above) then what's the
value of knowing it was one some time ago?

Fundamentally using refcount_read() in control flow is broken, it's a
very bad anti-pattern.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-11-18 10:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-18  3:53 [PATCH] refcount: introduce refcount_is_one() helper function Yajun Deng
2021-11-18  7:42 ` Peter Zijlstra
2021-11-18  8:12 ` yajun.deng
2021-11-18  8:44   ` Peter Zijlstra
2021-11-18 10:34   ` yajun.deng
2021-11-18 10:49     ` Peter Zijlstra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).