Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 19 Sep 2008 12:36:52 -0700
From:      Julian Elischer <julian@elischer.org>
To:        Marko Zec <zec@freebsd.org>
Cc:        Perforce Change Reviews <perforce@freebsd.org>, FreeBSD virtualization mailing list <freebsd-virtualization@freebsd.org>
Subject:   Re: vimage and curvnet.
Message-ID:  <48D3FF54.2010802@elischer.org>
In-Reply-To: <200809192023.42904.zec@freebsd.org>
References:  <200809191743.m8JHhZj8009388@repoman.freebsd.org> <48D3E778.3020305@elischer.org> <200809192023.42904.zec@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Marko Zec wrote:
> On Friday 19 September 2008 19:55:04 Julian Elischer wrote:
>> Marko Zec wrote:
>>> http://perforce.freebsd.org/chv.cgi?CH=150125
>>>
>>> Change 150125 by zec@zec_tpx32 on 2008/09/19 17:43:14
>>>
>>> 	curvnet is never set on entry to sysctl handlers, thus
>>> 	set the vnet context via TD_TO_VNET(curthread), because
>>> 	all threads must have a cred->vimage->vnet context set.
>> it seems to me that vnet is an important enough variable that we
>                       ^^^^
> I guess you meant curvnet?
> 
>> could put it in the pcpu structure and have the scheduler set it up
>> on context switches when vimage is compiled in.
> 
> Right now curvnet is defined in vimage.h as
> 
> #define curvnet curthread->td_vnet
> 
> Note that there's also a helper field in struct thread called 
> td_vnet_lpush aimed at tracing CURVNET_SET() stacking as well as 
> finding places when CURVNET_RESTORE() is not called after 
> CURVNET_SET().  Both td_vnet and td_vnet_lpush _must_ be stored with 
> each thread anyhow, given that threads can migrate practically at any 
> time.

not without the scheduler knowing about it they can't.

i.e. if the scheduler changes curvnet when it changes curthread
then it will always be correct.


> 
> So, if pcpu shadowing of curthread->td_vnet could yield performace 
> improvements than this would make sense,

It might, and it may remove some requirements for files to include proc.h

  but we would still need to
> keep td_vnet as a field in struct thread, and have CURVNET_SET() and 
> CURVNET_RESTORE() macros atomically update both the td_ field and its 
> pcpu shadow, which would require each CURVNET_XXX() macro to include 
> critical_enter() / _exit() invocations...

sure, well, if a running thread has curvnet in the pcpu field
and it is put back in the the thread struct on deschedule.
then what in td_vnet while the thread is running is not really important.

anyhow, it's just an idea.


> 
> 
>> thus it would always be directly available.
>> i.e. %gs:CURVNET_OFFSET  or whatever the syntax is on x86
>> and the equivalent on other architectures.
>>
>>> Affected files ...
>>>
>>> .. //depot/projects/vimage-commit2/src/sys/netinet/tcp_subr.c#14
>>> edit
>>>
>>> Differences ...
>>>
>>> ==== //depot/projects/vimage-commit2/src/sys/netinet/tcp_subr.c#14
>>> (text+ko) ====
>>>
>>> @@ -119,7 +119,7 @@
>>>  static int
>>>  sysctl_net_inet_tcp_mss_check(SYSCTL_HANDLER_ARGS)
>>>  {
>>> -	INIT_VNET_INET(curvnet);
>>> +	INIT_VNET_INET(TD_TO_VNET(curthread));
>>>  	int error, new;
>>>
>>>  	new = V_tcp_mssdflt;
>>> @@ -141,7 +141,7 @@
>>>  static int
>>>  sysctl_net_inet_tcp_mss_v6_check(SYSCTL_HANDLER_ARGS)
>>>  {
>>> -	INIT_VNET_INET6(curvnet);
>>> +	INIT_VNET_INET6(TD_TO_VNET(curthread));
>>>  	int error, new;
>>>
>>>  	new = V_tcp_v6mssdflt;
> 




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?48D3FF54.2010802>