On Wed, 11 Jan 2017 09:55:12 -0800 Tim Chen <[email protected]> wrote:
> From: "Huang, Ying" <[email protected]> > > This patch is to reduce the lock contention of swap_info_struct->lock > via using a more fine grained lock in swap_cluster_info for some swap > operations. swap_info_struct->lock is heavily contended if multiple > processes reclaim pages simultaneously. Because there is only one lock > for each swap device. While in common configuration, there is only one > or several swap devices in the system. The lock protects almost all > swap related operations. > > In fact, many swap operations only access one element of > swap_info_struct->swap_map array. And there is no dependency between > different elements of swap_info_struct->swap_map. So a fine grained > lock can be used to allow parallel access to the different elements of > swap_info_struct->swap_map. > > In this patch, one bit of swap_cluster_info is used as the bin spinlock > to protect the elements of swap_info_struct->swap_map in the swap > cluster and the fields of swap_cluster_info. This reduced locking > contention for swap_info_struct->swap_map access greatly. > > To use the bin spinlock, the size of swap_cluster_info needs to increase > from 4 bytes to 8 bytes on the 64bit system. This will use 4k more > memory for every 1G swap space. > > Because the size of swap_cluster_info is much smaller than the size of > the cache line (8 vs 64 on x86_64 architecture), there may be false > cache line sharing between swap_cluster_info bit spinlocks. To avoid > the false sharing in the first round of the swap cluster allocation, the > order of the swap clusters in the free clusters list is changed. So > that, the swap_cluster_info sharing the same cache line will be placed > as far as possible. After the first round of allocation, the order of > the clusters in free clusters list is expected to be random. So the > false sharing should be not noticeable. > > ... > > @@ -175,11 +175,16 @@ enum { > * protected by swap_info_struct.lock. > */ > struct swap_cluster_info { > - unsigned int data:24; > - unsigned int flags:8; > + unsigned long data; > }; > > ... > > +static inline void __lock_cluster(struct swap_cluster_info *ci) > +{ > + bit_spin_lock(CLUSTER_FLAG_LOCK_BIT, &ci->data); > +} hm, bit_spin_lock() is a nasty thing. It is slow and it doesn't have all the lockdep support. Would the world end if we added a spinlock to swap_cluster_info? Check my math: for each 1G of wapspace we have 256k pages, hence 1k of swap_cluster_infos, hence 4k of memory. ie, one page of memory for each 256,000 pages of swap. Is increasing that 1/256000 to 2/256000 a big deal? Also, I note that struct swap_cluster_info is only used in swapfile.c and as a cleanup we could move its definition into that .c file. Perhaps other things could be moved as well..
