Bug 54690

Summary: [PATCH] wrap swap_pager's swhash with a mutex
Product: Base System Reporter: Bruce M Simpson <bms>
Component: kernAssignee: Alan Cox <alc>
Status: Closed FIXED    
Severity: Affects Only Me    
Priority: Normal    
Version: 5.1-RELEASE   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
swhash.col.patch
none
swhash.col.patch
none
foo.txt none

Description Bruce M Simpson 2003-07-21 09:40:18 UTC 
	Requested and reviewed by alc@freebsd.org -- wrap the swap block
	hash table in the swap pager with a fine grained lock.
Comment 1 Mike Makonnen freebsd_committer freebsd_triage 2003-07-21 12:46:48 UTC 
Responsible Changed
From-To: freebsd-bugs->alc

This seems to be in your area Alan.
Comment 2 Bruce M Simpson 2003-07-23 05:40:46 UTC 
On Tue, Jul 22, 2003 at 03:21:08PM -0500, Alan L. Cox wrote:
> Briefly, the pointer returned by swp_pager_hash() is only valid for as
> long as the hash table mutex is held.  Thus, for insertion and deletion,
> swp_pager_hash() will need to return with the mutex held.
> 
> In other words, any place you see "*pswap" or "->swb_hnext", the mutex
> must be held.

No problem - to be perfectly honest I wasn't 100% sure about it, but it
looked OK. I'll walk through the pointer accesses by hand and arrive
at a correct result.

BMS
Comment 3 Bruce M Simpson 2003-07-23 10:49:08 UTC 
On Tue, Jul 22, 2003 at 03:21:08PM -0500, Alan L. Cox wrote:
> In other words, any place you see "*pswap" or "->swb_hnext", the mutex
> must be held.

Please see the attached patch.

There is a problem with this patch. When a process exists,
swap_pager_freespace() is called, and WITNESS reports a
lock order reversal between vm_object and swhash. I've attached
the DDB backtrace.

I don't quite understand what's going on, because I've looked at these
functions, and they all acquire the vm object lock first:-

swap_pager_getpages()   --> doesn't lock vm_object, but references it
swap_pager_dealloc()  --> entered with a vm_object lock, then takes swhash lock
swap_pager_freespace() --> entered with a vm_object lock

swp_pager_meta_build() --> is not called with vm_object lock

swp_pager_meta_ctl() -> not called with vm_object lock
   - does not attempt to obtain vm_object lock
   - GIANT_REQUIRED

swp_pager_meta_free() -> vm_object held
  - called from swap_pager_freespace()
  -> first call triggers 'lock order reversal' error in WITNESS

BMS
Comment 4 Alan Cox freebsd_committer freebsd_triage 2003-11-11 06:52:53 UTC 
State Changed
From-To: open->closed

A variation of this patch has been applied to -CURRENT.  Thanks, Bruce.