All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Kees Cook <keescook@chromium.org>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	David Miller <davem@davemloft.net>,
	"Reshetova, Elena" <elena.reshetova@intel.com>,
	Network Development <netdev@vger.kernel.org>,
	bridge@lists.linux-foundation.org,
	LKML <linux-kernel@vger.kernel.org>,
	Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>,
	James Morris <jmorris@namei.org>,
	Patrick McHardy <kaber@trash.net>,
	Stephen Hemminger <stephen@networkplumber.org>,
	Hans Liljestrand <ishkamiel@gmail.com>,
	David Windsor <dwindsor@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH 07/17] net: convert sock.sk_refcnt from atomic_t to refcount_t
Date: Wed, 22 Mar 2017 13:25:40 +0100	[thread overview]
Message-ID: <20170322122540.odnm4j3r4c4uf7wt@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <1490148199.16816.126.camel@edumazet-glaptop3.roam.corp.google.com>

On Tue, Mar 21, 2017 at 07:03:19PM -0700, Eric Dumazet wrote:

> Note that we might define two refcount_inc()  : One that does whole
> tests, and refcount_inc_relaxed() that might translate to atomic_inc()
> on non debug kernels.

So you'd want a duplicate interface, such that most code, which doesn't
care about refcount performance much, can still have all the tests
enabled.

But the code that cares about it (and preferably can prove it with
numbers) can use the other.

I'm also somewhat hesitant to use _relaxed for this distinction, as it
has a clear meaning in atomics, maybe _nocheck?

Also; what operations do you want _nocheck variants of, only
refcount_inc() ?

That said; I'm really loath to provide these without actual measurements
that prove they make a difference.

> Then later, maybe provide a dynamic infrastructure so that we can
> dynamically force the full checks even for refcount_inc_relaxed() on say
> 1% of the hosts, to get better debug coverage ?

Shouldn't be too hard to do in arch specific code using alternative
stuff. Generic code could use jump labels I suppose, but that would
result in bigger code.

WARNING: multiple messages have this Message-ID (diff)
From: Peter Zijlstra <peterz@infradead.org>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Kees Cook <keescook@chromium.org>,
	Patrick McHardy <kaber@trash.net>,
	Network Development <netdev@vger.kernel.org>,
	bridge@lists.linux-foundation.org,
	LKML <linux-kernel@vger.kernel.org>,
	James Morris <jmorris@namei.org>,
	Hans Liljestrand <ishkamiel@gmail.com>,
	Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>,
	Andrew Morton <akpm@linux-foundation.org>,
	David Windsor <dwindsor@gmail.com>,
	David Miller <davem@davemloft.net>,
	"Reshetova, Elena" <elena.reshetova@intel.com>,
	Herbert Xu <herbert@gondor.apana.org.au>
Subject: Re: [PATCH 07/17] net: convert sock.sk_refcnt from atomic_t to refcount_t
Date: Wed, 22 Mar 2017 13:25:40 +0100	[thread overview]
Message-ID: <20170322122540.odnm4j3r4c4uf7wt@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <1490148199.16816.126.camel@edumazet-glaptop3.roam.corp.google.com>

On Tue, Mar 21, 2017 at 07:03:19PM -0700, Eric Dumazet wrote:

> Note that we might define two refcount_inc()  : One that does whole
> tests, and refcount_inc_relaxed() that might translate to atomic_inc()
> on non debug kernels.

So you'd want a duplicate interface, such that most code, which doesn't
care about refcount performance much, can still have all the tests
enabled.

But the code that cares about it (and preferably can prove it with
numbers) can use the other.

I'm also somewhat hesitant to use _relaxed for this distinction, as it
has a clear meaning in atomics, maybe _nocheck?

Also; what operations do you want _nocheck variants of, only
refcount_inc() ?

That said; I'm really loath to provide these without actual measurements
that prove they make a difference.

> Then later, maybe provide a dynamic infrastructure so that we can
> dynamically force the full checks even for refcount_inc_relaxed() on say
> 1% of the hosts, to get better debug coverage ?

Shouldn't be too hard to do in arch specific code using alternative
stuff. Generic code could use jump labels I suppose, but that would
result in bigger code.

WARNING: multiple messages have this Message-ID (diff)
From: Peter Zijlstra <peterz@infradead.org>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Kees Cook <keescook@chromium.org>,
	Patrick McHardy <kaber@trash.net>,
	Network Development <netdev@vger.kernel.org>,
	bridge@lists.linux-foundation.org,
	LKML <linux-kernel@vger.kernel.org>,
	James Morris <jmorris@namei.org>,
	Hans Liljestrand <ishkamiel@gmail.com>,
	Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>,
	Andrew Morton <akpm@linux-foundation.org>,
	David Windsor <dwindsor@gmail.com>,
	David Miller <davem@davemloft.net>,
	"Reshetova, Elena" <elena.reshetova@intel.com>,
	Herbert Xu <herbert@gondor.apana.org.au>
Subject: Re: [Bridge] [PATCH 07/17] net: convert sock.sk_refcnt from atomic_t to refcount_t
Date: Wed, 22 Mar 2017 13:25:40 +0100	[thread overview]
Message-ID: <20170322122540.odnm4j3r4c4uf7wt@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <1490148199.16816.126.camel@edumazet-glaptop3.roam.corp.google.com>

On Tue, Mar 21, 2017 at 07:03:19PM -0700, Eric Dumazet wrote:

> Note that we might define two refcount_inc()  : One that does whole
> tests, and refcount_inc_relaxed() that might translate to atomic_inc()
> on non debug kernels.

So you'd want a duplicate interface, such that most code, which doesn't
care about refcount performance much, can still have all the tests
enabled.

But the code that cares about it (and preferably can prove it with
numbers) can use the other.

I'm also somewhat hesitant to use _relaxed for this distinction, as it
has a clear meaning in atomics, maybe _nocheck?

Also; what operations do you want _nocheck variants of, only
refcount_inc() ?

That said; I'm really loath to provide these without actual measurements
that prove they make a difference.

> Then later, maybe provide a dynamic infrastructure so that we can
> dynamically force the full checks even for refcount_inc_relaxed() on say
> 1% of the hosts, to get better debug coverage ?

Shouldn't be too hard to do in arch specific code using alternative
stuff. Generic code could use jump labels I suppose, but that would
result in bigger code.


  reply	other threads:[~2017-03-22 12:26 UTC|newest]

Thread overview: 135+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-16 15:28 [PATCH 00/17] net subsystem refcount conversions Elena Reshetova
2017-03-16 15:28 ` [Bridge] " Elena Reshetova
2017-03-16 15:28 ` [PATCH 01/17] net: convert neighbour.refcnt from atomic_t to refcount_t Elena Reshetova
2017-03-16 15:28   ` [Bridge] " Elena Reshetova
2017-03-16 15:28 ` [PATCH 02/17] net: convert neigh_params.refcnt " Elena Reshetova
2017-03-16 15:28   ` [Bridge] " Elena Reshetova
2017-03-16 15:28 ` [PATCH 03/17] net: convert nf_bridge_info.use " Elena Reshetova
2017-03-16 15:28   ` [Bridge] " Elena Reshetova
2017-03-16 15:28 ` [PATCH 04/17] net: convert sk_buff.users " Elena Reshetova
2017-03-16 15:28   ` [Bridge] " Elena Reshetova
2017-03-16 15:28 ` [PATCH 05/17] net: convert sk_buff_fclones.fclone_ref " Elena Reshetova
2017-03-16 15:28   ` [Bridge] " Elena Reshetova
2017-03-16 15:28 ` [PATCH 06/17] net: convert sock.sk_wmem_alloc " Elena Reshetova
2017-03-16 15:28   ` [Bridge] " Elena Reshetova
2017-03-16 15:28 ` [PATCH 07/17] net: convert sock.sk_refcnt " Elena Reshetova
2017-03-16 15:28   ` [Bridge] " Elena Reshetova
2017-03-16 16:58   ` Eric Dumazet
2017-03-16 16:58     ` [Bridge] " Eric Dumazet
2017-03-16 16:58     ` Eric Dumazet
2017-03-16 17:38     ` Kees Cook
2017-03-16 17:38       ` [Bridge] " Kees Cook
2017-03-16 19:10       ` David Miller
2017-03-16 19:10         ` [Bridge] " David Miller
2017-03-16 19:10         ` David Miller
2017-03-17  7:42         ` Reshetova, Elena
2017-03-17  7:42           ` [Bridge] " Reshetova, Elena
2017-03-17  7:42           ` Reshetova, Elena
2017-03-17 16:13           ` Eric Dumazet
2017-03-17 16:13             ` [Bridge] " Eric Dumazet
2017-03-17 16:13             ` Eric Dumazet
2017-03-18 16:47             ` Herbert Xu
2017-03-18 16:47               ` [Bridge] " Herbert Xu
2017-03-19  1:21               ` David Miller
2017-03-19  1:21                 ` [Bridge] " David Miller
2017-03-19  1:21                 ` David Miller
2017-03-20 10:39                 ` Peter Zijlstra
2017-03-20 10:39                   ` [Bridge] " Peter Zijlstra
2017-03-20 13:16                   ` Herbert Xu
2017-03-20 13:16                     ` [Bridge] " Herbert Xu
2017-03-20 13:16                     ` Herbert Xu
2017-03-20 13:23                     ` Peter Zijlstra
2017-03-20 13:23                       ` [Bridge] " Peter Zijlstra
2017-03-20 13:27                       ` Herbert Xu
2017-03-20 13:27                         ` [Bridge] " Herbert Xu
2017-03-20 13:27                         ` Herbert Xu
2017-03-20 13:40                         ` Peter Zijlstra
2017-03-20 13:40                           ` [Bridge] " Peter Zijlstra
2017-03-20 14:51                           ` Eric Dumazet
2017-03-20 14:51                             ` [Bridge] " Eric Dumazet
2017-03-20 14:51                             ` Eric Dumazet
2017-03-20 14:59                             ` Eric Dumazet
2017-03-20 14:59                               ` [Bridge] " Eric Dumazet
2017-03-20 14:59                               ` Eric Dumazet
2017-03-20 16:18                               ` Eric Dumazet
2017-03-20 16:18                                 ` [Bridge] " Eric Dumazet
2017-03-20 16:18                                 ` Eric Dumazet
2017-03-20 16:34                                 ` Eric Dumazet
2017-03-20 16:34                                   ` [Bridge] " Eric Dumazet
2017-03-20 14:59                             ` Peter Zijlstra
2017-03-20 14:59                               ` [Bridge] " Peter Zijlstra
2017-03-21 20:49                           ` Kees Cook
2017-03-21 20:49                             ` [Bridge] " Kees Cook
2017-03-21 20:49                             ` Kees Cook
2017-03-21 21:23                             ` Eric Dumazet
2017-03-21 21:23                               ` [Bridge] " Eric Dumazet
2017-03-21 21:23                               ` Eric Dumazet
2017-03-21 22:36                               ` David Miller
2017-03-21 22:36                                 ` [Bridge] " David Miller
2017-03-21 22:36                                 ` David Miller
2017-03-21 23:51                               ` Kees Cook
2017-03-21 23:51                                 ` [Bridge] " Kees Cook
2017-03-21 23:51                                 ` Kees Cook
2017-03-22  2:03                                 ` Eric Dumazet
2017-03-22  2:03                                   ` [Bridge] " Eric Dumazet
2017-03-22 12:25                                   ` Peter Zijlstra [this message]
2017-03-22 12:25                                     ` Peter Zijlstra
2017-03-22 12:25                                     ` Peter Zijlstra
2017-03-22 13:22                                     ` Eric Dumazet
2017-03-22 13:22                                       ` [Bridge] " Eric Dumazet
2017-03-22 13:22                                       ` Eric Dumazet
2017-03-22 14:33                                       ` Peter Zijlstra
2017-03-22 14:33                                         ` [Bridge] " Peter Zijlstra
2017-03-22 14:33                                         ` Peter Zijlstra
2017-03-22 14:54                                         ` Eric Dumazet
2017-03-22 14:54                                           ` [Bridge] " Eric Dumazet
2017-03-22 14:54                                           ` Eric Dumazet
2017-03-22 15:08                                           ` Peter Zijlstra
2017-03-22 15:08                                             ` [Bridge] " Peter Zijlstra
2017-03-22 15:08                                             ` Peter Zijlstra
2017-03-22 15:22                                             ` Eric Dumazet
2017-03-22 15:22                                               ` [Bridge] " Eric Dumazet
2017-03-22 15:22                                               ` Eric Dumazet
2017-03-22 16:51                                           ` Peter Zijlstra
2017-03-22 16:51                                             ` [Bridge] " Peter Zijlstra
2017-03-22 16:51                                             ` Peter Zijlstra
2017-03-22 19:08                                   ` Kees Cook
2017-03-22 19:08                                     ` [Bridge] " Kees Cook
2017-03-22 19:08                                     ` Kees Cook
2017-03-22 12:11                                 ` Peter Zijlstra
2017-03-22 12:11                                   ` [Bridge] " Peter Zijlstra
2017-03-22 12:11                                   ` Peter Zijlstra
2017-03-20 14:10                     ` David Laight
2017-03-20 14:10                       ` [Bridge] " David Laight
2017-03-20 14:10                       ` David Laight
2017-03-20 14:28                       ` Peter Zijlstra
2017-03-20 14:28                         ` [Bridge] " Peter Zijlstra
2017-03-20 14:28                         ` Peter Zijlstra
2017-03-20 15:00                         ` David Laight
2017-03-20 15:00                           ` [Bridge] " David Laight
2017-03-20 15:00                           ` David Laight
2017-03-16 15:28 ` [PATCH 08/17] net: convert sk_filter.refcnt " Elena Reshetova
2017-03-16 15:28   ` [Bridge] " Elena Reshetova
2017-03-16 16:04   ` Daniel Borkmann
2017-03-16 16:04     ` [Bridge] " Daniel Borkmann
2017-03-17  8:02     ` Reshetova, Elena
2017-03-17  8:02       ` [Bridge] " Reshetova, Elena
2017-03-17  8:02       ` Reshetova, Elena
2017-03-16 15:28 ` [PATCH 09/17] net: convert ip_mc_list.refcnt " Elena Reshetova
2017-03-16 15:28   ` [Bridge] " Elena Reshetova
2017-03-16 15:29 ` [PATCH 10/17] net: convert in_device.refcnt " Elena Reshetova
2017-03-16 15:29   ` [Bridge] " Elena Reshetova
2017-03-16 15:29 ` [PATCH 11/17] net: convert netpoll_info.refcnt " Elena Reshetova
2017-03-16 15:29   ` [Bridge] " Elena Reshetova
2017-03-16 15:29 ` [PATCH 12/17] net: convert unix_address.refcnt " Elena Reshetova
2017-03-16 15:29   ` [Bridge] " Elena Reshetova
2017-03-16 15:29 ` [PATCH 13/17] net: convert fib_rule.refcnt " Elena Reshetova
2017-03-16 15:29   ` [Bridge] " Elena Reshetova
2017-03-16 15:29 ` [PATCH 14/17] net: convert inet_frag_queue.refcnt " Elena Reshetova
2017-03-16 15:29   ` [Bridge] " Elena Reshetova
2017-03-16 15:29 ` [PATCH 15/17] net: convert net.passive " Elena Reshetova
2017-03-16 15:29   ` [Bridge] " Elena Reshetova
2017-03-16 15:29 ` [PATCH 16/17] net: convert netlbl_lsm_cache.refcount " Elena Reshetova
2017-03-16 15:29   ` [Bridge] " Elena Reshetova
2017-03-16 15:29 ` [PATCH 17/17] net: convert packet_fanout.sk_ref " Elena Reshetova
2017-03-16 15:29   ` [Bridge] " Elena Reshetova

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170322122540.odnm4j3r4c4uf7wt@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=bridge@lists.linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=dwindsor@gmail.com \
    --cc=elena.reshetova@intel.com \
    --cc=eric.dumazet@gmail.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=ishkamiel@gmail.com \
    --cc=jmorris@namei.org \
    --cc=kaber@trash.net \
    --cc=keescook@chromium.org \
    --cc=kuznet@ms2.inr.ac.ru \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.