On 11/11/2014 09:58 PM, Kelley Nielsen wrote:
> The prototype of the new swapoff (without the quadratic complexity)
> presently ignores the frontswap case. Pass the count of
> pages_to_unuse down the page table walks in try_to_unuse(),
> and return from the walk when the desired number of pages
> has been swapped back in.
>
> Signed-off-by: Kelley Nielsen <kelleynnn@xxxxxxxxx>
> ---
> mm/shmem.c | 1 +
> mm/swapfile.c | 53 +++++++++++++++++++++++++++++++++++++----------------
> 2 files changed, 38 insertions(+), 16 deletions(-)
>
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 2a7179c..e7a813f 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -629,6 +629,7 @@ static int shmem_unuse_inode(struct inode *inode, unsigned int type)
> int entries = 0;
> swp_entry_t entry;
> unsigned int stype;
> +
> pgoff_t start = 0;
Why is there an shmem.c blank line in the frontswap patch?
> @@ -1210,6 +1212,15 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
> SetPageDirty(page);
> unlock_page(page);
> page_cache_release(page);
> + if (ret && pages_to_unuse > 0) {
> + pages_to_unuse--;
> + /*
> + * we've unused all we need for frontswap,
> + * so return special code to indicate this.
> + */
> + if (pages_to_unuse == 0)
> + return 2;
> + }
If you are using a magic value, could you make it a #define so
people can more easily find out why the code is testing for == 2
elsewhere?
One obvious bug is that the pages_to_unuse variable is passed by
value, so try_to_unuse never sees that unuse_pte_range decremented
the counter. You will want to use a pointer instead.
A second issue is that you decrement pages_to_unuse on every pte
unmap, and not on every swap slot that is unused. Would it make
more sense to decrement pages_to_unuse where you call
delete_from_swap_cache?
Other than that, this series looks good to me.
--
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>