sched_lock && thread_lock()

[Jeff Roberson]

On Mon, 21 May 2007, Bruce Evans wrote:

> On Sun, 20 May 2007, Jeff Roberson wrote:
>
>> Attilio and I have been working on addressing the increasing problem of 
>> sched_lock contention on -CURRENT.  Attilio has been addressing the parts 
>> of the kernel which do not need to fall under the scheduler lock and moving 
>> them into seperate locks.  For example, the ldt/gdt lock and clock lock 
>> which were committed earlier.  Also, using atomics for the vmcnt structure.
>
> Using atomics in the vmmeter struct is mostly just a pessimization and
> and obfuscation, since locks are still needed for accesses to more
> than one variable at a time.  For these cases, locks are needed for

You are right, there are some cases which this pessimized.  I wanted to 
make sure the cnt members that previously were protected by the sched_lock 
were still correct.  However, I overlooked some of these which were 
accessed many at a time.  What should happen is we should find out if any 
locks do protect the remaining members and if so, don't use VMCNT*, but 
mark the header describing how they are protected.

> correctness, and are also probably more efficient if there are > 2
> accesses.  Then lock poisoning requires the same locks to be held for
> updates of the variables (to prevent variables changing while you are
> using them), and atomic updates of single variables are a pessimization
> too.  The VMCNT*() ineterface encorages this pessimization and has other
> problems (messes with volatile, and not actually doing atomic accesses
> for VMCNT_GET()).  Some of the cases where locks aren't needed (mainly
> for userland-only statistics) were already handled better by
> PCPU_LAZY_INC().  Unfortunately, the method used in PCPU_LAZY_INC()
> doesn't work for variables that the kernel wants to access as globals.

Depending on the architecture PCPU may be more expensive than atomics so 
we decied not to use it all over.

I'll revisit the places with multiple accesses with Attilio and clean this 
up.

Thanks,
Jeff

>
> Example of code with multiple accesses:
>
> % /*
> %  * Return the number of pages we need to free-up or cache
> %  * A positive number indicates that we do not have enough free pages.
> %  */
> % % static __inline % int
> % vm_paging_target(void)
> % {
> %     return (
> % 	(VMCNT_GET(free_target) + VMCNT_GET(cache_min)) - % 
> (VMCNT_GET(free_count) + VMCNT_GET(cache_count))
> %     );
> % }
>
> Without holding a lock throughout this, this returns a garbage value.
> Without holding a lock throughout this and the actions taken depending
> on the return value, garbage actions may be taken.  Values that are
> only slightly wrong might work OK, but this is not clear, and if it
> does work then volatile variables and [non-]atomic accesses to the
> variables to get it.
>
> Bruce
>