cvs commit: src/sys/i386/i386 sys_machdep.c

[John Baldwin]

On Friday 12 September 2008 05:51:11 am Konstantin Belousov wrote:
> kib         2008-09-12 09:51:11 UTC
> 
>   FreeBSD src repository
> 
>   Modified files:
>     sys/i386/i386        sys_machdep.c 
>   Log:
>   SVN rev 182960 on 2008-09-12 09:51:11Z by kib
>   
>   The user_ldt_alloc() function shall return with dt_lock locked.
>   The user_ldt_free() function shall return with dt_lock unlocked.
>   Error handling code in both functions do not handle this, fix it by
>   doing necessary lock/unlock.
>   
>   While there, fix minor style nits.

Hmm, I had actually thought it was intentional for user_ldt_alloc() to only 
return with the lock held on success and depend on a later call to another 
method to drop the lock in the success case (so the locking isn't visible to 
consumers of the API in theory).  For example, i386_ldt_grow() depended on 
this feature and is now broken (it leaks a lock on failure).  I missed this 
when looking at this yesterday.

Other notes:

- Since user_ldt_free() handles the case of there not being an LDT, the code
  in exec_setregs() on i386 can be simplified to just always call
  user_ldt_free().
- cpu_exit() could possibly do the same.  I wonder if exec_setregs() needs the
  same fixup to %gs that cpu_exit() does.  If so, that could possibly be moved
  into user_ldt_free().  Ah, exec_setregs() does it unconditionally.  I think
  you could make cpu_exit() just do it unconditionally as well before calling
  user_ldt_free().

-- 
John Baldwin