mm/swap: Regular page swap optimizations

Change Log:
v4:
1. Fix a bug in unlock cluster in add_swap_count_continuation(). We should use unlock_cluster() instead of unlock_cluser_or_swap_info().
2. During swap off, handle race when swap slot is marked unused but allocated, and not yet placed in swap cache. Wait for swap slot to be placed in swap cache and not abort swap off.
3. Initialize n_ret in get_swap_pages().

v3:
1. Fix bug that didn't check for page already in swap cache before skipping read ahead and return null page.
2. Fix bug that didn't try to allocate from global pool if allocation from swap slot cache did not succeed.
3. Fix memory allocation bug for spaces to store split up 64MB radix tree
4. Fix problems caused by races between get_swap_page, cpu online/offline and swap_on/off

v2:
1. Fix bug in the index limit used in scan_swap_map_try_ssd_cluster when searching for empty slots in cluster.
2. Fix bug in swap off that incorrectly determines if we still have swap devices left.
3. Port patches to mmotm-2016-10-11-15-46 branch

Andrew,

We're updating this patch series with some minor fixes. Please consider this patch series for inclusion to 4.10.

Times have changed. Coming generation of Solid state Block device latencies are getting down to sub 100 usec, which is within an order of magnitude of DRAM, and their performance is orders of magnitude higher than the single- spindle rotational media we've swapped to historically.

This could benefit many usage scenearios. For example cloud providers who overcommit their memory (as VM don't use all the memory provisioned). Having a fast swap will allow them to be more aggressive in memory overcommit and fit more VMs to a platform.

In our testing [see footnote], the median latency that the kernel adds to a page fault is 15 usec, which comes quite close to the amount that will be contributed by the underlying I/O devices.

The software latency comes mostly from contentions on the locks protecting the radix tree of the swap cache and also the locks protecting the individual swap devices. The lock contentions already consumed 35% of cpu cycles in our test. In the very near future, software latency will become the bottleneck to swap performnace as block device I/O latency gets within the shouting distance of DRAM speed.

This patch set, plus a previous patch Ying already posted (commit: f6498b3f) reduced the median page fault latency from 15 usec to 4 usec (375% reduction) for DRAM based pmem block device.

Patch 1 is a clean up patch.

Patch 2 creates a lock per cluster, this gives us a more fine graind lock
that can be used for accessing swap_map, and not lock the whole swap device

Patch 3 splits the swap cache radix tree into 64MB chunks, reducing
the rate that we have to contende for the radix tree.

Patch 4 eliminates unnecessary page allocation for read ahead.

Patch 5-9 create a per cpu cache of the swap slots, so we don't have
to contend on the swap device to get a swap slot or to release a swap slot. And we allocate and release the swap slots in batches for better efficiency.

Hi,

Quoted 52 lines · Tim Chen

Change Log:
v4:
1. Fix a bug in unlock cluster in add_swap_count_continuation(). We should use unlock_cluster() instead of unlock_cluser_or_swap_info().
2. During swap off, handle race when swap slot is marked unused but allocated, and not yet placed in swap cache. Wait for swap slot to be placed in swap cache and not abort swap off.
3. Initialize n_ret in get_swap_pages().

v3:
1. Fix bug that didn't check for page already in swap cache before skipping read ahead and return null page.
2. Fix bug that didn't try to allocate from global pool if allocation from swap slot cache did not succeed.
3. Fix memory allocation bug for spaces to store split up 64MB radix tree
4. Fix problems caused by races between get_swap_page, cpu online/offline and swap_on/off

v2:
1. Fix bug in the index limit used in scan_swap_map_try_ssd_cluster when searching for empty slots in cluster.
2. Fix bug in swap off that incorrectly determines if we still have swap devices left.
3. Port patches to mmotm-2016-10-11-15-46 branch

Andrew,

We're updating this patch series with some minor fixes. Please consider this patch series for inclusion to 4.10.

Times have changed. Coming generation of Solid state Block device latencies are getting down to sub 100 usec, which is within an order of magnitude of DRAM, and their performance is orders of magnitude higher than the single- spindle rotational media we've swapped to historically.

This could benefit many usage scenearios. For example cloud providers who overcommit their memory (as VM don't use all the memory provisioned). Having a fast swap will allow them to be more aggressive in memory overcommit and fit more VMs to a platform.

In our testing [see footnote], the median latency that the kernel adds to a page fault is 15 usec, which comes quite close to the amount that will be contributed by the underlying I/O devices.

The software latency comes mostly from contentions on the locks protecting the radix tree of the swap cache and also the locks protecting the individual swap devices. The lock contentions already consumed 35% of cpu cycles in our test. In the very near future, software latency will become the bottleneck to swap performnace as block device I/O latency gets within the shouting distance of DRAM speed.

Quoted 4 lines

This patch set, plus a previous patch Ying already posted (commit: f6498b3f) reduced the median page fault latency from 15 usec to 4 usec (375% reduction) for DRAM based pmem block device.

The patchset has used several techniqueus to reduce lock contention, for example, batching alloc/free, fine-grained lock and cluster distribution to avoid cache false-sharing. Each items has different complexity and benefits so could you show the number for each step of pathchset? It would be better to include the nubmer in each description. It helps how the patch is important when we consider complexitiy of the patch.

Quoted 2 lines

Patch 1 is a clean up patch.

Could it be separated patch?

Quoted 3 lines

Patch 2 creates a lock per cluster, this gives us a more fine graind lock
that can be used for accessing swap_map, and not lock the whole swap device

I hope you make three steps to review easier. You can create some functions like swap_map_lock and cluster_lock which are wrapper functions just hold swap_lock. It doesn't change anything performance pov but it clearly shows what kinds of lock we should use in specific context.

Then, you can introduce more fine-graind lock in next patch and apply it into those wrapper functions.

And last patch, you can adjust cluster distribution to avoid false-sharing. And the description should include how it's bad in testing so it's worth.

Frankly speaking, although I'm huge user of bit_spin_lock(zram/zsmalloc have used it heavily), I don't like swap subsystem uses it.
During zram development, it really hurts debugging due to losing lockdep. The reason zram have used it is by size concern of embedded world but server would be not critical so please consider trade-off of spinlock vs. bit_spin_lock.

Quoted 2 lines

Patch 3 splits the swap cache radix tree into 64MB chunks, reducing
the rate that we have to contende for the radix tree.

To me, it's rather hacky. I think it might be common problem for page cache so can we think another generalized way like range_lock? Ccing Jan.

Quoted 1 line

Patch 4 eliminates unnecessary page allocation for read ahead.

Could it be separated patch?

Quoted 4 lines

Patch 5-9 create a per cpu cache of the swap slots, so we don't have
to contend on the swap device to get a swap slot or to release a swap slot. And we allocate and release the swap slots in batches for better efficiency.

To me, idea is good although I feel the amount of code is rather huge and messy so it should include the number about the benefit, at least.

And it might make some of patches in this patchset if we put this batching ahead before other patches redundant.

Sorry for vague commenting. In this phase, it's really hard to review.

Quoted 6 lines

The patchset has used several techniqueus to reduce lock contention, for example, batching alloc/free, fine-grained lock and cluster distribution to avoid cache false-sharing. Each items has different complexity and benefits so could you show the number for each step of pathchset? It would be better to include the nubmer in each description. It helps how the patch is important when we consider complexitiy of the patch.

One common problem of scalability optimization is that, after you have optimized one lock, the end result may be not very good, because another lock becomes heavily contended. Similar problem occurs here, there are mainly two locks during swap out/in, one protects swap cache, the other protects swap device. We can achieve good scalability only after having optimized the two locks.

You cannot say that one patch is not important just because the test result for that single patch is not very good. Because without that, the end result of the whole series will be not very good.

Quoted 18 lines

Quoted 3 lines

Patch 1 is a clean up patch.

Could it be separated patch?

Quoted 3 lines

Patch 2 creates a lock per cluster, this gives us a more fine graind lock
that can be used for accessing swap_map, and not lock the whole swap device

I hope you make three steps to review easier. You can create some functions like swap_map_lock and cluster_lock which are wrapper functions just hold swap_lock. It doesn't change anything performance pov but it clearly shows what kinds of lock we should use in specific context.

Then, you can introduce more fine-graind lock in next patch and apply it into those wrapper functions.

Quoted 8 lines

And last patch, you can adjust cluster distribution to avoid false-sharing. And the description should include how it's bad in testing so it's worth.

Frankly speaking, although I'm huge user of bit_spin_lock(zram/zsmalloc have used it heavily), I don't like swap subsystem uses it.
During zram development, it really hurts debugging due to losing lockdep. The reason zram have used it is by size concern of embedded world but server would be not critical so please consider trade-off of spinlock vs. bit_spin_lock.

There will be one struct swap_cluster_info for every 1MB swap space. So, for example, for 1TB swap space, the number of struct swap_cluster_info will be one million. To reduce the RAM usage, we choose to use bit_spin_lock, otherwise, spinlock is better. The code will be used by embedded, PC and server, so the RAM usage is important.

Hi Huang,

On Wed, Dec 28, 2016 at 09:54:27AM +0800, Huang, Ying wrote:

< snip >

Quoted 7 lines

Quoted 6 lines

The patchset has used several techniqueus to reduce lock contention, for example, batching alloc/free, fine-grained lock and cluster distribution to avoid cache false-sharing. Each items has different complexity and benefits so could you show the number for each step of pathchset? It would be better to include the nubmer in each description. It helps how the patch is important when we consider complexitiy of the patch.

Quoted 6 lines

One common problem of scalability optimization is that, after you have optimized one lock, the end result may be not very good, because another lock becomes heavily contended. Similar problem occurs here, there are mainly two locks during swap out/in, one protects swap cache, the other protects swap device. We can achieve good scalability only after having optimized the two locks.

Yes. You can describe that situation into the description. For example, "with this patch, we can watch less swap_lock contention with perf but overall performance is not good because swap cache lock still is still contended heavily like below data so next patch will solve the problem".

It will make patch's justficiation clear.

Quoted 4 lines

You cannot say that one patch is not important just because the test result for that single patch is not very good. Because without that, the end result of the whole series will be not very good.

I know that but this patchset are lack of number too much to justify each works. You can show just raw number itself of a techniqueue although it is not huge benefit or even worse. You can explain the reason why it was not good, which would be enough motivation for next patch.

Number itself wouldn't be important but justfication is really crucial to review/merge patchset and number will help it a lot in especially MM community.

Quoted 27 lines

Quoted 26 lines

Quoted 3 lines

Patch 1 is a clean up patch.

Could it be separated patch?

Quoted 3 lines

Patch 2 creates a lock per cluster, this gives us a more fine graind lock
that can be used for accessing swap_map, and not lock the whole swap device

I hope you make three steps to review easier. You can create some functions like swap_map_lock and cluster_lock which are wrapper functions just hold swap_lock. It doesn't change anything performance pov but it clearly shows what kinds of lock we should use in specific context.

Then, you can introduce more fine-graind lock in next patch and apply it into those wrapper functions.

And last patch, you can adjust cluster distribution to avoid false-sharing. And the description should include how it's bad in testing so it's worth.

Frankly speaking, although I'm huge user of bit_spin_lock(zram/zsmalloc have used it heavily), I don't like swap subsystem uses it.
During zram development, it really hurts debugging due to losing lockdep. The reason zram have used it is by size concern of embedded world but server would be not critical so please consider trade-off of spinlock vs. bit_spin_lock.

Quoted 5 lines

There will be one struct swap_cluster_info for every 1MB swap space. So, for example, for 1TB swap space, the number of struct swap_cluster_info will be one million. To reduce the RAM usage, we choose to use bit_spin_lock, otherwise, spinlock is better. The code will be used by embedded, PC and server, so the RAM usage is important.

It seems you already increase swap_cluster_info 4 byte to support bit_spin_lock.

Compared to that, how much memory does spin_lock increase?

Quoted 68 lines · Minchan Kim

Hi Huang,

On Wed, Dec 28, 2016 at 09:54:27AM +0800, Huang, Ying wrote:

< snip >

Quoted 7 lines

Quoted 6 lines

The patchset has used several techniqueus to reduce lock contention, for example, batching alloc/free, fine-grained lock and cluster distribution to avoid cache false-sharing. Each items has different complexity and benefits so could you show the number for each step of pathchset? It would be better to include the nubmer in each description. It helps how the patch is important when we consider complexitiy of the patch.

Quoted 6 lines

One common problem of scalability optimization is that, after you have optimized one lock, the end result may be not very good, because another lock becomes heavily contended. Similar problem occurs here, there are mainly two locks during swap out/in, one protects swap cache, the other protects swap device. We can achieve good scalability only after having optimized the two locks.

Yes. You can describe that situation into the description. For example, "with this patch, we can watch less swap_lock contention with perf but overall performance is not good because swap cache lock still is still contended heavily like below data so next patch will solve the problem".

It will make patch's justficiation clear.

Quoted 4 lines

You cannot say that one patch is not important just because the test result for that single patch is not very good. Because without that, the end result of the whole series will be not very good.

I know that but this patchset are lack of number too much to justify each works. You can show just raw number itself of a techniqueue although it is not huge benefit or even worse. You can explain the reason why it was not good, which would be enough motivation for next patch.

Number itself wouldn't be important but justfication is really crucial to review/merge patchset and number will help it a lot in especially MM community.

Quoted 27 lines

Quoted 26 lines

Quoted 3 lines

Patch 1 is a clean up patch.

Could it be separated patch?

Quoted 3 lines

Patch 2 creates a lock per cluster, this gives us a more fine graind lock
that can be used for accessing swap_map, and not lock the whole swap device

I hope you make three steps to review easier. You can create some functions like swap_map_lock and cluster_lock which are wrapper functions just hold swap_lock. It doesn't change anything performance pov but it clearly shows what kinds of lock we should use in specific context.

Then, you can introduce more fine-graind lock in next patch and apply it into those wrapper functions.

And last patch, you can adjust cluster distribution to avoid false-sharing. And the description should include how it's bad in testing so it's worth.

Frankly speaking, although I'm huge user of bit_spin_lock(zram/zsmalloc have used it heavily), I don't like swap subsystem uses it.
During zram development, it really hurts debugging due to losing lockdep. The reason zram have used it is by size concern of embedded world but server would be not critical so please consider trade-off of spinlock vs. bit_spin_lock.

Quoted 8 lines

Quoted 5 lines

There will be one struct swap_cluster_info for every 1MB swap space. So, for example, for 1TB swap space, the number of struct swap_cluster_info will be one million. To reduce the RAM usage, we choose to use bit_spin_lock, otherwise, spinlock is better. The code will be used by embedded, PC and server, so the RAM usage is important.

It seems you already increase swap_cluster_info 4 byte to support bit_spin_lock.

The increment only occurs on 64bit platform. On 32bit platform, the size is the same as before.

Quoted 1 line

Compared to that, how much memory does spin_lock increase?

The size of struct swap_cluster_info will increase from 4 bytes to 16 bytes on 64bit platform. I guess it will increase from 4 bytes to 8 bytes on 32bit platform at least, but I did not test that.

Quoted 20 lines

Quoted 6 lines

Quoted 6 lines

Quoted 5 lines

Frankly speaking, although I'm huge user of bit_spin_lock(zram/zsmalloc have used it heavily), I don't like swap subsystem uses it.
During zram development, it really hurts debugging due to losing lockdep. The reason zram have used it is by size concern of embedded world but server would be not critical so please consider trade-off of spinlock vs. bit_spin_lock.

Quoted 8 lines

Quoted 5 lines

There will be one struct swap_cluster_info for every 1MB swap space. So, for example, for 1TB swap space, the number of struct swap_cluster_info will be one million. To reduce the RAM usage, we choose to use bit_spin_lock, otherwise, spinlock is better. The code will be used by embedded, PC and server, so the RAM usage is important.

It seems you already increase swap_cluster_info 4 byte to support bit_spin_lock.

The increment only occurs on 64bit platform. On 32bit platform, the size is the same as before.

Quoted 1 line

Compared to that, how much memory does spin_lock increase?

Quoted 3 lines

The size of struct swap_cluster_info will increase from 4 bytes to 16 bytes on 64bit platform. I guess it will increase from 4 bytes to 8 bytes on 32bit platform at least, but I did not test that.

Sorry, I make a mistake during test. The size of struct swap_cluster_info will increase from 4 bytes to 8 bytes on 64 bit platform. I think it will increase from 4 bytes to 8 bytes on 32 bit platform too (not tested).

On Wed, Dec 28, 2016 at 11:31:06AM +0800, Huang, Ying wrote:

< snip >

Quoted 24 lines

Quoted 23 lines

Quoted 6 lines

Quoted 6 lines

Quoted 5 lines

Frankly speaking, although I'm huge user of bit_spin_lock(zram/zsmalloc have used it heavily), I don't like swap subsystem uses it.
During zram development, it really hurts debugging due to losing lockdep. The reason zram have used it is by size concern of embedded world but server would be not critical so please consider trade-off of spinlock vs. bit_spin_lock.

Quoted 8 lines

Quoted 5 lines

There will be one struct swap_cluster_info for every 1MB swap space. So, for example, for 1TB swap space, the number of struct swap_cluster_info will be one million. To reduce the RAM usage, we choose to use bit_spin_lock, otherwise, spinlock is better. The code will be used by embedded, PC and server, so the RAM usage is important.

It seems you already increase swap_cluster_info 4 byte to support bit_spin_lock.

The increment only occurs on 64bit platform. On 32bit platform, the size is the same as before.

Quoted 1 line

Compared to that, how much memory does spin_lock increase?

The size of struct swap_cluster_info will increase from 4 bytes to 16 bytes on 64bit platform. I guess it will increase from 4 bytes to 8 bytes on 32bit platform at least, but I did not test that.

Quoted 4 lines

Sorry, I make a mistake during test. The size of struct swap_cluster_info will increase from 4 bytes to 8 bytes on 64 bit platform. I think it will increase from 4 bytes to 8 bytes on 32 bit platform too (not tested).

Thanks for the information.
To me, it's not big when we consider spinlock's usefullness which helps cache-line bouncing, lockdep and happy with RT people.

So, I vote spin_lock but I'm not in charge of deciding on that and your opinion might be different still. If so, let's pass the decision to maintainer.

Instead, please write down above content in description for maintainer to judge it fairly.

Quoted 2 lines

Patch 3 splits the swap cache radix tree into 64MB chunks, reducing
the rate that we have to contende for the radix tree.

To me, it's rather hacky. I think it might be common problem for page cache so can we think another generalized way like range_lock? Ccing Jan.

Hi,

Quoted 5 lines · Minchan Kim

Quoted 2 lines

Patch 3 splits the swap cache radix tree into 64MB chunks, reducing
the rate that we have to contende for the radix tree.

To me, it's rather hacky. I think it might be common problem for page cache so can we think another generalized way like range_lock? Ccing Jan.

I agree on the hackyness of the patch and that page cache would suffer with the same contention (although the files are usually smaller than swap so it would not be that visible I guess). But I don't see how range lock would help here - we need to serialize modifications of the tree structure itself and that is difficult to achieve with the range lock. So what you would need is either a different data structure for tracking swap cache entries or a finer grained locking of the radix tree.

Hi Jan,

Quoted 9 lines · Jan Kara

Hi,

Quoted 5 lines · Minchan Kim

Quoted 2 lines

Patch 3 splits the swap cache radix tree into 64MB chunks, reducing
the rate that we have to contende for the radix tree.

To me, it's rather hacky. I think it might be common problem for page cache so can we think another generalized way like range_lock? Ccing Jan.

Quoted 7 lines

I agree on the hackyness of the patch and that page cache would suffer with the same contention (although the files are usually smaller than swap so it would not be that visible I guess). But I don't see how range lock would help here - we need to serialize modifications of the tree structure itself and that is difficult to achieve with the range lock. So what you would need is either a different data structure for tracking swap cache entries or a finer grained locking of the radix tree.

Thanks for the comment, Jan.

I think there are more general options. One is to shrink batching pages like Mel and Tim had approached.

https://patchwork.kernel.org/patch/9008421/
https://patchwork.kernel.org/patch/9322793/

Or concurrent page cache by peter.

https://www.kernel.org/doc/ols/2007/ols2007v2-pages-311-318.pdf

Ccing Nick who might have an interest on lockless page cache.

Thanks.

Hi, Minchan,

Quoted 20 lines · Minchan Kim

Hi Jan,

Quoted 9 lines · Jan Kara

Hi,

Quoted 5 lines · Minchan Kim

Quoted 2 lines

Patch 3 splits the swap cache radix tree into 64MB chunks, reducing
the rate that we have to contende for the radix tree.

To me, it's rather hacky. I think it might be common problem for page cache so can we think another generalized way like range_lock? Ccing Jan.

Quoted 7 lines

I agree on the hackyness of the patch and that page cache would suffer with the same contention (although the files are usually smaller than swap so it would not be that visible I guess). But I don't see how range lock would help here - we need to serialize modifications of the tree structure itself and that is difficult to achieve with the range lock. So what you would need is either a different data structure for tracking swap cache entries or a finer grained locking of the radix tree.

Quoted 7 lines

Thanks for the comment, Jan.

I think there are more general options. One is to shrink batching pages like Mel and Tim had approached.

https://patchwork.kernel.org/patch/9008421/
https://patchwork.kernel.org/patch/9322793/

This helps to reduce the lock contention on radix tree of swap cache. But splitting swap cache has much better performance. So we switched from that solution to current solution.

Quoted 2 lines

Or concurrent page cache by peter.

Quoted 1 line

https://www.kernel.org/doc/ols/2007/ols2007v2-pages-311-318.pdf

I think this is good, it helps swap and file cache. But I don't know whether other people want to go this way and how much effort will be needed.

In contrast, splitting swap cache is quite simple, for implementation and review. And the effect is good.

Hi Jan,

Quoted 10 lines · Jan Kara

Hi,

Quoted 6 lines · Minchan Kim

Quoted 4 lines

Patch 3 splits the swap cache radix tree into 64MB chunks, reducing A A A A A A A A the rate that we have to contende for the radix tree.

To me, it's rather hacky. I think it might be common problem for page cache so can we think another generalized way like range_lock? Ccing Jan.

Quoted 7 lines

I agree on the hackyness of the patch and that page cache would suffer with the same contention (although the files are usually smaller than swap so it would not be that visible I guess). But I don't see how range lock would help here - we need to serialize modifications of the tree structure itself and that is difficult to achieve with the range lock. So what you would need is either a different data structure for tracking swap cache entries or a finer grained locking of the radix tree.

Thanks for the comment, Jan.

I think there are more general options. One is to shrink batching pages like Mel and Tim had approached.

https://patchwork.kernel.org/patch/9008421/
https://patchwork.kernel.org/patch/9322793/

Or concurrent page cache by peter.

https://www.kernel.org/doc/ols/2007/ols2007v2-pages-311-318.pdf

Ccing Nick who might have an interest on lockless page cache.

Thanks.

The patchset has used several techniqueus to reduce lock contention, for example, batching alloc/free, fine-grained lock and cluster distribution to avoid cache false-sharing. Each items has different complexity and benefits so could you show the number for each step of pathchset? It would be better to include the nubmer in each description. It helps how the patch is important when we consider complexitiy of the patch.

Quoted 1 line

Patch 1 is a clean up patch.

Could it be separated patch?

Quoted 3 lines

Patch 2 creates a lock per cluster, this gives us a more fine graind lock
that can be used for accessing swap_map, and not lock the whole swap device

Quoted 2 lines

Patch 3 splits the swap cache radix tree into 64MB chunks, reducing
the rate that we have to contende for the radix tree.

Quoted 2 lines

Patch 4 eliminates unnecessary page allocation for read ahead.

Could it be separated patch?

Quoted 4 lines

Patch 5-9 create a per cpu cache of the swap slots, so we don't have
to contend on the swap device to get a swap slot or to release a swap slot. And we allocate and release the swap slots in batches for better efficiency.

Hi, Minchan,

Minchan Kim <[email protected]> writes:
[snip]

Quoted 1 line

Quoted 6 lines

The patchset has used several techniqueus to reduce lock contention, for example, batching alloc/free, fine-grained lock and cluster distribution to avoid cache false-sharing. Each items has different complexity and benefits so could you show the number for each step of pathchset? It would be better to include the nubmer in each description. It helps how the patch is important when we consider complexitiy of the patch.

Here is the test data.

We test the vm-scalability swap-w-seq test case with 32 processes on a Xeon E5 v3 system. The swap device used is a RAM simulated PMEM (persistent memory) device. To test the sequential swapping out, the test case created 32 processes, which sequentially allocate and write to the anonymous pages until the RAM and part of the swap device is used up.

The patchset is rebased on v4.9-rc8. So the baseline performance is as follow,

"vmstat.swap.so": 1428002,

"perf-profile.calltrace.cycles-pp._raw_spin_lock_irq.__add_to_swap_cache.add_to_swap_cache.add_to_swap.shrink_page_list": 13.94, "perf-profile.calltrace.cycles-pp._raw_spin_lock_irqsave.__remove_mapping.shrink_page_list.shrink_inactive_list.shrink_node_memcg": 13.75, "perf-profile.calltrace.cycles-pp._raw_spin_lock.swap_info_get.swapcache_free.__remove_mapping.shrink_page_list": 7.05, "perf-profile.calltrace.cycles-pp._raw_spin_lock.swap_info_get.page_swapcount.try_to_free_swap.swap_writepage": 7.03, "perf-profile.calltrace.cycles-pp._raw_spin_lock.__swap_duplicate.swap_duplicate.try_to_unmap_one.rmap_walk_anon": 7.02, "perf-profile.calltrace.cycles-pp._raw_spin_lock.get_swap_page.add_to_swap.shrink_page_list.shrink_inactive_list": 6.83, "perf-profile.calltrace.cycles-pp._raw_spin_lock.page_check_address_transhuge.page_referenced_one.rmap_walk_anon.rmap_walk": 0.81,

Quoted 8 lines

Quoted 2 lines

Patch 1 is a clean up patch.

Could it be separated patch?

Quoted 3 lines

Patch 2 creates a lock per cluster, this gives us a more fine graind lock
that can be used for accessing swap_map, and not lock the whole swap device

After patch 2, the result is as follow,

"vmstat.swap.so": 1481704, "perf-profile.calltrace.cycles-pp._raw_spin_lock_irq.__add_to_swap_cache.add_to_swap_cache.add_to_swap.shrink_page_list": 27.53, "perf-profile.calltrace.cycles-pp._raw_spin_lock_irqsave.__remove_mapping.shrink_page_list.shrink_inactive_list.shrink_node_memcg": 27.01, "perf-profile.calltrace.cycles-pp._raw_spin_lock.free_pcppages_bulk.drain_pages_zone.drain_pages.drain_local_pages": 1.03,

The swap out throughput is at the same level, but the lock contention on swap_info_struct->lock is eliminated.

Quoted 3 lines

Quoted 2 lines

Patch 3 splits the swap cache radix tree into 64MB chunks, reducing
the rate that we have to contende for the radix tree.

After patch 3,

"vmstat.swap.so": 2050097, "perf-profile.calltrace.cycles-pp._raw_spin_lock.get_swap_page.add_to_swap.shrink_page_list.shrink_inactive_list": 43.27, "perf-profile.calltrace.cycles-pp._raw_spin_lock.get_page_from_freelist.__alloc_pages_nodemask.alloc_pages_vma.handle_mm_fault": 4.84,

The swap out throughput is improved about ~43% compared with baseline. The lock contention on swap cache radix tree lock is eliminated. swap_info_struct->lock in get_swap_page() becomes the most heavy contended lock.

Quoted 10 lines

Quoted 3 lines

Patch 4 eliminates unnecessary page allocation for read ahead.

Could it be separated patch?

Quoted 4 lines

Patch 5-9 create a per cpu cache of the swap slots, so we don't have
to contend on the swap device to get a swap slot or to release a swap slot. And we allocate and release the swap slots in batches for better efficiency.

After patch 9,

"vmstat.swap.so": 4170746, "perf-profile.calltrace.cycles-pp._raw_spin_lock.swapcache_free_entries.free_swap_slot.free_swap_and_cache.unmap_page_range": 13.91, "perf-profile.calltrace.cycles-pp._raw_spin_lock.get_page_from_freelist.__alloc_pages_nodemask.alloc_pages_vma.handle_mm_fault": 8.56, "perf-profile.calltrace.cycles-pp._raw_spin_lock.get_page_from_freelist.__alloc_pages_slowpath.__alloc_pages_nodemask.alloc_pages_vma": 2.56, "perf-profile.calltrace.cycles-pp._raw_spin_lock.get_swap_pages.get_swap_page.add_to_swap.shrink_page_list": 2.47,

The swap out throughput is improved about 192% compared with the baseline. There are still some lock contention for swap_info_struct->lock, but the pressure begins to shift to buddy system now.

Best Regards,
Huang, Ying

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Hi,

Quoted 5 lines · Ying

Hi, Minchan,

Minchan Kim <[email protected]> writes:
[snip]

Quoted 1 line

Quoted 8 lines

Quoted 6 lines

The patchset has used several techniqueus to reduce lock contention, for example, batching alloc/free, fine-grained lock and cluster distribution to avoid cache false-sharing. Each items has different complexity and benefits so could you show the number for each step of pathchset? It would be better to include the nubmer in each description. It helps how the patch is important when we consider complexitiy of the patch.

Here is the test data.

Thanks!

Quoted 8 lines

We test the vm-scalability swap-w-seq test case with 32 processes on a Xeon E5 v3 system. The swap device used is a RAM simulated PMEM (persistent memory) device. To test the sequential swapping out, the test case created 32 processes, which sequentially allocate and write to the anonymous pages until the RAM and part of the swap device is used up.

Quoted 4 lines

The patchset is rebased on v4.9-rc8. So the baseline performance is as follow,

"vmstat.swap.so": 1428002,

What does it mean? vmstat.pswpout?

Quoted 8 lines

"perf-profile.calltrace.cycles-pp._raw_spin_lock_irq.__add_to_swap_cache.add_to_swap_cache.add_to_swap.shrink_page_list": 13.94, "perf-profile.calltrace.cycles-pp._raw_spin_lock_irqsave.__remove_mapping.shrink_page_list.shrink_inactive_list.shrink_node_memcg":
13.75,
"perf-profile.calltrace.cycles-pp._raw_spin_lock.swap_info_get.swapcache_free.__remove_mapping.shrink_page_list": 7.05, "perf-profile.calltrace.cycles-pp._raw_spin_lock.swap_info_get.page_swapcount.try_to_free_swap.swap_writepage": 7.03, "perf-profile.calltrace.cycles-pp._raw_spin_lock.__swap_duplicate.swap_duplicate.try_to_unmap_one.rmap_walk_anon": 7.02, "perf-profile.calltrace.cycles-pp._raw_spin_lock.get_swap_page.add_to_swap.shrink_page_list.shrink_inactive_list": 6.83, "perf-profile.calltrace.cycles-pp._raw_spin_lock.page_check_address_transhuge.page_referenced_one.rmap_walk_anon.rmap_walk": 0.81,

Numbers mean overhead percentage reported by perf?

Quoted 25 lines

Quoted 7 lines

Quoted 1 line

Patch 1 is a clean up patch.

Could it be separated patch?

Quoted 3 lines

Patch 2 creates a lock per cluster, this gives us a more fine graind lock
that can be used for accessing swap_map, and not lock the whole swap device

After patch 2, the result is as follow,

"vmstat.swap.so": 1481704, "perf-profile.calltrace.cycles-pp._raw_spin_lock_irq.__add_to_swap_cache.add_to_swap_cache.add_to_swap.shrink_page_list": 27.53, "perf-profile.calltrace.cycles-pp._raw_spin_lock_irqsave.__remove_mapping.shrink_page_list.shrink_inactive_list.shrink_node_memcg":
27.01,
"perf-profile.calltrace.cycles-pp._raw_spin_lock.free_pcppages_bulk.drain_pages_zone.drain_pages.drain_local_pages": 1.03,

The swap out throughput is at the same level, but the lock contention on swap_info_struct->lock is eliminated.

Quoted 3 lines

Quoted 2 lines

Patch 3 splits the swap cache radix tree into 64MB chunks, reducing
the rate that we have to contende for the radix tree.

After patch 3,

Quoted 8 lines

"vmstat.swap.so": 2050097, "perf-profile.calltrace.cycles-pp._raw_spin_lock.get_swap_page.add_to_swap.shrink_page_list.shrink_inactive_list": 43.27, "perf-profile.calltrace.cycles-pp._raw_spin_lock.get_page_from_freelist.__alloc_pages_nodemask.alloc_pages_vma.handle_mm_fault": 4.84,

The swap out throughput is improved about ~43% compared with baseline. The lock contention on swap cache radix tree lock is eliminated. swap_info_struct->lock in get_swap_page() becomes the most heavy contended lock.

The numbers are great! Please include those into each patchset. And I ask one more thing I said earlier about patch 2.

""
I hope you make three steps to review easier. You can create some functions like swap_map_lock and cluster_lock which are wrapper functions just hold swap_lock. It doesn't change anything performance pov but it clearly shows what kinds of lock we should use in specific context.

Then, you can introduce more fine-graind lock in next patch and apply it into those wrapper functions.

And last patch, you can adjust cluster distribution to avoid false-sharing. And the description should include how it's bad in testing so it's worth. ""

It makes review more easier, I believe.

Quoted 25 lines

Quoted 10 lines

Quoted 3 lines

Patch 4 eliminates unnecessary page allocation for read ahead.

Could it be separated patch?

Quoted 4 lines

Patch 5-9 create a per cpu cache of the swap slots, so we don't have
to contend on the swap device to get a swap slot or to release a swap slot. And we allocate and release the swap slots in batches for better efficiency.

After patch 9,

"vmstat.swap.so": 4170746, "perf-profile.calltrace.cycles-pp._raw_spin_lock.swapcache_free_entries.free_swap_slot.free_swap_and_cache.unmap_page_range": 13.91, "perf-profile.calltrace.cycles-pp._raw_spin_lock.get_page_from_freelist.__alloc_pages_nodemask.alloc_pages_vma.handle_mm_fault": 8.56, "perf-profile.calltrace.cycles-pp._raw_spin_lock.get_page_from_freelist.__alloc_pages_slowpath.__alloc_pages_nodemask.alloc_pages_vma":
2.56,
"perf-profile.calltrace.cycles-pp._raw_spin_lock.get_swap_pages.get_swap_page.add_to_swap.shrink_page_list": 2.47,

The swap out throughput is improved about 192% compared with the baseline. There are still some lock contention for swap_info_struct->lock, but the pressure begins to shift to buddy system now.

Quoted 8 lines

Best Regards,
Huang, Ying

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>