Re: [PATCH v2] mm: prototype: rid swapoff of quadratic complexity

From: Vineeth Remanan Pillai
Date: Mon Nov 26 2018 - 13:35:36 EST


Hi Mathew,


Thanks for your response!

On 11/26/18 12:22 PM, Matthew Wilcox wrote:
On Mon, Nov 26, 2018 at 04:55:21PM +0000, Vineeth Remanan Pillai wrote:
+ do {
+ XA_STATE(xas, &mapping->i_pages, start);
+ int i;
+ int entries = 0;
+ struct page *page;
+ pgoff_t indices[PAGEVEC_SIZE];
+ unsigned long end = start + PAGEVEC_SIZE;
+ rcu_read_lock();
+ xas_for_each(&xas, page, end) {
I think this is a mistake. You should probably specify ULONG_MAX for the
end. Otherwise if there are no swap entries in the first 60kB of the file,
you'll just exit. That does mean you'll need to check 'entries' for
hitting PAGEVEC_SIZE.

Thanks for pointing this out. I shall fix this in the next version.

This seems terribly complicated. You run through i_pages, record the
indices of the swap entries, then go back and look them up again by
calling shmem_getpage() which calls the incredibly complex 300 line
shmem_getpage_gfp().

Can we refactor shmem_getpage_gfp() to skip some of the checks which
aren't necessary when called from this path, and turn this into a nice
simple xas_for_each() loop which works one entry at a time?

I shall investigate this and make this simpler as you suggested.

+ list_for_each_safe(p, next, &shmem_swaplist) {
+ info = list_entry(p, struct shmem_inode_info, swaplist);
This could use list_for_each_entry_safe(), right?

Yes, you are right. Will fix.


Thanks,

Vineeth