netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ian Campbell <Ian.Campbell@citrix.com>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: "Wei Liu (Intern)" <wei.liu2@citrix.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
	"konrad.wilk@oracle.com" <konrad.wilk@oracle.com>
Subject: Re: [RFC PATCH V4 02/13] netback: add module unload function.
Date: Thu, 2 Feb 2012 20:50:21 +0000	[thread overview]
Message-ID: <1328215821.13189.24.camel@dagon.hellion.org.uk> (raw)
In-Reply-To: <1328214866.2480.18.camel@edumazet-laptop>

On Thu, 2012-02-02 at 20:34 +0000, Eric Dumazet wrote:
> Le jeudi 02 février 2012 à 19:59 +0000, Ian Campbell a écrit :
> > On Thu, 2012-02-02 at 17:48 +0000, Eric Dumazet wrote:
> > > Le jeudi 02 février 2012 à 17:28 +0000, Wei Liu a écrit :
> > > 
> > > > You're right about this.
> > > > 
> > > > But this part is destined to get wiped out (in the very near future?) --
> > > > see following patches. So I don't think it is worthy to fix this.
> > > > 
> > > 
> > > Before adding new bugs, you must fix previous ones.
> > 
> > I've never heard of this requirement before! It's a wonder anyone ever
> > gets anything done.
> > 
> > Anyway, I think it would be reasonable to just remove the kthread_bind
> > call from this loop. We don't actually want/need a thread per online CPU
> > in any strict sense, we just want there to be some number of worker
> > threads available and ~numcpus at start of day is a good enough
> > approximation for that number. There have been patches floating around
> > in the past which make the number of groups a module parameter which
> > would also be a reasonable thing to dig out if we weren't just about to
> > remove all this code anyway.
> > 
> > So removing the kthread_bind is good enough for the short term, and for
> > stable if people feel that is necessary, and we can continue in mainline
> > with the direction Wei's patches are taking us.
> > 
> 
> That sounds a right fix.
>
> Why do think its not reasonable that I ask a bug fix ?

I don't think it is at all unreasonable to ask for bug fixes but in this
case Wei's series is removing the code in question (which would also
undoubtedly fix the bug).

As it happens the fix turns out to be simple but if it were complex I
would perhaps have disagreed more strongly about spending effort fixing
code that is removed 2 patches later, although obviously that would have
depended on the specifics of the fix in that case.

> Next time, dont bother send patches for review if you dont want
> reviewers.

The review which you are doing is certainly very much appreciated, I'm
sorry if my disagreement over this one point gave/gives the impression
that it is not.

Ian.

  parent reply	other threads:[~2012-02-02 20:50 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-02 16:49 [RFC PATCH V4] Xen netback / netfront improvement Wei Liu
2012-02-02 16:49 ` [RFC PATCH V4 01/13] netback: page pool version 1 Wei Liu
2012-02-02 17:26   ` Eric Dumazet
2012-02-17 19:19     ` Konrad Rzeszutek Wilk
2012-02-20 16:26       ` Wei Liu
2012-02-02 16:49 ` [RFC PATCH V4 02/13] netback: add module unload function Wei Liu
2012-02-02 17:08   ` Eric Dumazet
2012-02-02 17:28     ` Wei Liu
2012-02-02 17:48       ` Eric Dumazet
2012-02-02 19:59         ` Ian Campbell
2012-02-02 20:34           ` Eric Dumazet
2012-02-02 20:37             ` Eric Dumazet
2012-02-02 20:50             ` Ian Campbell [this message]
2012-02-02 22:52               ` Paul Gortmaker
2012-02-03  6:38                 ` Ian Campbell
2012-02-03  7:25                   ` Eric Dumazet
2012-02-03  8:02                     ` Ian Campbell
2012-02-03 11:27                     ` Wei Liu
2012-02-02 16:49 ` [RFC PATCH V4 03/13] netback: add module get/put operations along with vif connect/disconnect Wei Liu
2012-02-02 16:49 ` [RFC PATCH V4 04/13] netback: switch to NAPI + kthread model Wei Liu
2012-02-02 16:49 ` [RFC PATCH V4 05/13] netback: switch to per-cpu scratch space Wei Liu
2012-02-02 16:49 ` [RFC PATCH V4 06/13] netback: melt xen_netbk into xenvif Wei Liu
2012-02-02 16:49 ` [RFC PATCH V4 07/13] netback: alter internal function/structure names Wei Liu
2012-02-02 16:49 ` [RFC PATCH V4 08/13] xenbus_client: extend interface to support mapping / unmapping of multi page ring Wei Liu
2012-02-03 16:55   ` Konrad Rzeszutek Wilk
2012-02-03 17:20     ` Wei Liu
2012-02-03 17:35       ` Konrad Rzeszutek Wilk
2012-02-06 17:21       ` Konrad Rzeszutek Wilk
2012-02-06 17:30         ` Wei Liu
2012-02-02 16:49 ` [RFC PATCH V4 09/13] Bundle fix for xen backends and frontends Wei Liu
2012-02-03  2:34   ` Konrad Rzeszutek Wilk
2012-02-02 16:49 ` [RFC PATCH V4 10/13] netback: multi page ring support Wei Liu
2012-02-02 16:49 ` [RFC PATCH V4 11/13] netback: split event channels support Wei Liu
2012-02-02 16:49 ` [RFC PATCH V4 12/13] netfront: multi page ring support Wei Liu
2012-02-15 22:42   ` Konrad Rzeszutek Wilk
2012-02-15 22:52     ` David Miller
2012-02-15 23:53       ` Konrad Rzeszutek Wilk
2012-02-16 10:02     ` Wei Liu
2012-02-16 10:16       ` Wei Liu
2012-02-17 15:10         ` Konrad Rzeszutek Wilk
2012-02-16 22:57       ` Konrad Rzeszutek Wilk
2012-02-02 16:49 ` [RFC PATCH V4 13/13] netfront: split event channels support Wei Liu

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=1328215821.13189.24.camel@dagon.hellion.org.uk \
    --to=ian.campbell@citrix.com \
    --cc=eric.dumazet@gmail.com \
    --cc=konrad.wilk@oracle.com \
    --cc=netdev@vger.kernel.org \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xensource.com \
    /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 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).