On Wed, 11 Jan 2017 09:55:12 -0800 Tim Chen <tim.c.chen@xxxxxxxxxxxxxxx> wrote:
> From: "Huang, Ying" <ying.huang@xxxxxxxxx>
>
> 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..
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>