diff mbox

[1/8] netback: don't bind kthread to cpu

Message ID 1360944010-15336-2-git-send-email-wei.liu2@citrix.com
State Not Applicable, archived
Delegated to: David Miller
Headers show

Commit Message

Wei Liu Feb. 15, 2013, 4 p.m. UTC
The initialization process makes an assumption that the online cpus are
numbered from 0 to xen_netbk_group_nr-1,  which is not always true.

As we only need a pool of worker threads, simply don't bind them to specific
cpus.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 drivers/net/xen-netback/netback.c |    2 --
 1 file changed, 2 deletions(-)

Comments

Konrad Rzeszutek Wilk March 4, 2013, 8:51 p.m. UTC | #1
On Fri, Feb 15, 2013 at 04:00:02PM +0000, Wei Liu wrote:
> The initialization process makes an assumption that the online cpus are
> numbered from 0 to xen_netbk_group_nr-1,  which is not always true.

And xen_netbk_group_nr is num_online_cpus()?

So under what conditions does this change? Is this when the CPU hotplug
is involved and the CPUs go offline? In which case should there be a
CPU hotplug notifier to re-bind the workers are appropiate?

> 
> As we only need a pool of worker threads, simply don't bind them to specific
> cpus.

OK. Is there another method of doing this? Are there patches to make the thread
try to be vCPU->guest affinite?

> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  drivers/net/xen-netback/netback.c |    2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index 3ae49b1..db8d45a 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -1729,8 +1729,6 @@ static int __init netback_init(void)
>  			goto failed_init;
>  		}
>  
> -		kthread_bind(netbk->task, group);
> -
>  		INIT_LIST_HEAD(&netbk->net_schedule_list);
>  
>  		spin_lock_init(&netbk->net_schedule_list_lock);
> -- 
> 1.7.10.4
> 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wei Liu March 5, 2013, 1:30 p.m. UTC | #2
On Mon, 2013-03-04 at 20:51 +0000, Konrad Rzeszutek Wilk wrote:
> On Fri, Feb 15, 2013 at 04:00:02PM +0000, Wei Liu wrote:
> > The initialization process makes an assumption that the online cpus are
> > numbered from 0 to xen_netbk_group_nr-1,  which is not always true.
> 
> And xen_netbk_group_nr is num_online_cpus()?
> 

Yes.

> So under what conditions does this change? Is this when the CPU hotplug
> is involved and the CPUs go offline? In which case should there be a

Yes, the hotplug path.

> CPU hotplug notifier to re-bind the workers are appropiate?
> 
> > 
> > As we only need a pool of worker threads, simply don't bind them to specific
> > cpus.
> 
> OK. Is there another method of doing this? Are there patches to make the thread
> try to be vCPU->guest affinite?
> 

No, not at the moment.


Wei.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Konrad Rzeszutek Wilk March 5, 2013, 1:56 p.m. UTC | #3
On Tue, Mar 05, 2013 at 01:30:10PM +0000, Wei Liu wrote:
> On Mon, 2013-03-04 at 20:51 +0000, Konrad Rzeszutek Wilk wrote:
> > On Fri, Feb 15, 2013 at 04:00:02PM +0000, Wei Liu wrote:
> > > The initialization process makes an assumption that the online cpus are
> > > numbered from 0 to xen_netbk_group_nr-1,  which is not always true.
> > 
> > And xen_netbk_group_nr is num_online_cpus()?
> > 
> 
> Yes.
> 
> > So under what conditions does this change? Is this when the CPU hotplug
> > is involved and the CPUs go offline? In which case should there be a
> 
> Yes, the hotplug path.
> 
> > CPU hotplug notifier to re-bind the workers are appropiate?

?
Can't that option be explored?
> > 
> > > 
> > > As we only need a pool of worker threads, simply don't bind them to specific
> > > cpus.
> > 
> > OK. Is there another method of doing this? Are there patches to make the thread
> > try to be vCPU->guest affinite?
> > 
> 
> No, not at the moment.
> 
> 
> Wei.
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wei Liu March 5, 2013, 2:04 p.m. UTC | #4
On Tue, 2013-03-05 at 13:56 +0000, Konrad Rzeszutek Wilk wrote:
> On Tue, Mar 05, 2013 at 01:30:10PM +0000, Wei Liu wrote:
> > On Mon, 2013-03-04 at 20:51 +0000, Konrad Rzeszutek Wilk wrote:
> > > On Fri, Feb 15, 2013 at 04:00:02PM +0000, Wei Liu wrote:
> > > > The initialization process makes an assumption that the online cpus are
> > > > numbered from 0 to xen_netbk_group_nr-1,  which is not always true.
> > > 
> > > And xen_netbk_group_nr is num_online_cpus()?
> > > 
> > 
> > Yes.
> > 
> > > So under what conditions does this change? Is this when the CPU hotplug
> > > is involved and the CPUs go offline? In which case should there be a
> > 
> > Yes, the hotplug path.
> > 
> > > CPU hotplug notifier to re-bind the workers are appropiate?
> 
> ?
> Can't that option be explored?

Thinking about it. Should be doable.


Wei.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Vrabel March 5, 2013, 2:42 p.m. UTC | #5
On 05/03/13 13:56, Konrad Rzeszutek Wilk wrote:
> On Tue, Mar 05, 2013 at 01:30:10PM +0000, Wei Liu wrote:
>> On Mon, 2013-03-04 at 20:51 +0000, Konrad Rzeszutek Wilk wrote:
>>> On Fri, Feb 15, 2013 at 04:00:02PM +0000, Wei Liu wrote:
>>>> The initialization process makes an assumption that the online cpus are
>>>> numbered from 0 to xen_netbk_group_nr-1,  which is not always true.
>>>
>>> And xen_netbk_group_nr is num_online_cpus()?
>>>
>>
>> Yes.
>>
>>> So under what conditions does this change? Is this when the CPU hotplug
>>> is involved and the CPUs go offline? In which case should there be a
>>
>> Yes, the hotplug path.
>>
>>> CPU hotplug notifier to re-bind the workers are appropiate?
> 
> ?
> Can't that option be explored?

I'm not sure binding netback threads to particular VCPUs is useful
without also binding the events to the corresponding VCPUs.

I would hope that the scheduler would tend to the correct behavior if
threads aren't bound, anyway.

David
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Konrad Rzeszutek Wilk March 5, 2013, 3:52 p.m. UTC | #6
On Tue, Mar 05, 2013 at 02:42:26PM +0000, David Vrabel wrote:
> On 05/03/13 13:56, Konrad Rzeszutek Wilk wrote:
> > On Tue, Mar 05, 2013 at 01:30:10PM +0000, Wei Liu wrote:
> >> On Mon, 2013-03-04 at 20:51 +0000, Konrad Rzeszutek Wilk wrote:
> >>> On Fri, Feb 15, 2013 at 04:00:02PM +0000, Wei Liu wrote:
> >>>> The initialization process makes an assumption that the online cpus are
> >>>> numbered from 0 to xen_netbk_group_nr-1,  which is not always true.
> >>>
> >>> And xen_netbk_group_nr is num_online_cpus()?
> >>>
> >>
> >> Yes.
> >>
> >>> So under what conditions does this change? Is this when the CPU hotplug
> >>> is involved and the CPUs go offline? In which case should there be a
> >>
> >> Yes, the hotplug path.
> >>
> >>> CPU hotplug notifier to re-bind the workers are appropiate?
> > 
> > ?
> > Can't that option be explored?
> 
> I'm not sure binding netback threads to particular VCPUs is useful
> without also binding the events to the corresponding VCPUs.
> 
> I would hope that the scheduler would tend to the correct behavior if
> threads aren't bound, anyway.

That is fine too. If that is what we want we just need to make the git
commit message be more descriptive of why we don't want to bind
to VCPUs.
> 
> David
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 3ae49b1..db8d45a 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -1729,8 +1729,6 @@  static int __init netback_init(void)
 			goto failed_init;
 		}
 
-		kthread_bind(netbk->task, group);
-
 		INIT_LIST_HEAD(&netbk->net_schedule_list);
 
 		spin_lock_init(&netbk->net_schedule_list_lock);