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 15:33:29 +0100	[thread overview]
Message-ID: <20170322143329.ya5jnretfptf4iud@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <1490188936.16816.143.camel@edumazet-glaptop3.roam.corp.google.com>

On Wed, Mar 22, 2017 at 06:22:16AM -0700, Eric Dumazet wrote:

> But admittedly we can replace all these by standard refcount_inc() and
> simply provide a CONFIG option to turn off the checks, and let brave
> people enable this option.

Still brings us back to lacking a real reason to provide that CONFIG
option. Not to mention that this CONFIG knob will kill the warnings for
everything, even the code that might not be as heavily audited as
network and which doesn't really care much about the performance of
refcount operations.


So I'm actually in favour of _nocheck variants, if we can show the need
for them. And I like your idea of being able to dynamically switch them
back to full debug as well.


But I would feel a whole lot better about the entire thing if we could
measure their impact. It would also give us good precedent to whack
other potential users of _nocheck over the head with -- show numbers.

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 15:33:29 +0100	[thread overview]
Message-ID: <20170322143329.ya5jnretfptf4iud@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <1490188936.16816.143.camel@edumazet-glaptop3.roam.corp.google.com>

On Wed, Mar 22, 2017 at 06:22:16AM -0700, Eric Dumazet wrote:

> But admittedly we can replace all these by standard refcount_inc() and
> simply provide a CONFIG option to turn off the checks, and let brave
> people enable this option.

Still brings us back to lacking a real reason to provide that CONFIG
option. Not to mention that this CONFIG knob will kill the warnings for
everything, even the code that might not be as heavily audited as
network and which doesn't really care much about the performance of
refcount operations.


So I'm actually in favour of _nocheck variants, if we can show the need
for them. And I like your idea of being able to dynamically switch them
back to full debug as well.


But I would feel a whole lot better about the entire thing if we could
measure their impact. It would also give us good precedent to whack
other potential users of _nocheck over the head with -- show numbers.

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 15:33:29 +0100	[thread overview]
Message-ID: <20170322143329.ya5jnretfptf4iud@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <1490188936.16816.143.camel@edumazet-glaptop3.roam.corp.google.com>

On Wed, Mar 22, 2017 at 06:22:16AM -0700, Eric Dumazet wrote:

> But admittedly we can replace all these by standard refcount_inc() and
> simply provide a CONFIG option to turn off the checks, and let brave
> people enable this option.

Still brings us back to lacking a real reason to provide that CONFIG
option. Not to mention that this CONFIG knob will kill the warnings for
everything, even the code that might not be as heavily audited as
network and which doesn't really care much about the performance of
refcount operations.


So I'm actually in favour of _nocheck variants, if we can show the need
for them. And I like your idea of being able to dynamically switch them
back to full debug as well.


But I would feel a whole lot better about the entire thing if we could
measure their impact. It would also give us good precedent to whack
other potential users of _nocheck over the head with -- show numbers.

  reply	other threads:[~2017-03-22 14:34 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
2017-03-22 12:25                                     ` [Bridge] " 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 [this message]
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=20170322143329.ya5jnretfptf4iud@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.