vmscan: Kill hibernation specific reclaim logic and unify it
in [Kernel]
|
Prev: wpa2 hangs v2.6.32-rc5-402-gb6727b1. Revert 7d930bc33653d5592dc386a76a38f39c2e962344 fixed it.
Next: [PATCH 1/1] MM: slqb, fix per_cpu access
From: KOSAKI Motohiro on 3 Nov 2009 09:10 > On Monday 02 November 2009, KOSAKI Motohiro wrote: > > > > Then, This patch changed shrink_all_memory() to only the wrapper function of > > > > do_try_to_free_pages(). it bring good reviewability and debuggability, and solve > > > > above problems. > > > > > > > > side note: Reclaim logic unificication makes two good side effect. > > > > - Fix recursive reclaim bug on shrink_all_memory(). > > > > it did forgot to use PF_MEMALLOC. it mean the system be able to stuck into deadlock. > > > > - Now, shrink_all_memory() got lockdep awareness. it bring good debuggability. > > > > > > As I said previously, I don't really see a reason to keep shrink_all_memory(). > > > > > > Do you think that removing it will result in performance degradation? > > > > Hmm... > > Probably, I misunderstood your mention. I thought you suggested to kill > > all hibernation specific reclaim code. I did. It's no performance degression. > > (At least, I didn't observe) > > > > But, if you hope to kill shrink_all_memory() function itsef, the short answer is, > > it's impossible. > > > > Current VM reclaim code need some preparetion to caller, and there are existing in > > both alloc_pages_slowpath() and try_to_free_pages(). We can't omit its preparation. > > Well, my grepping for 'shrink_all_memory' throughout the entire kernel source > code seems to indicate that hibernate_preallocate_memory() is the only current > user of it. I may be wrong, but I doubt it, unless some new users have been > added since 2.6.31. > > In case I'm not wrong, it should be safe to drop it from > hibernate_preallocate_memory(), because it's there for performance reasons > only. Now, since hibernate_preallocate_memory() appears to be the only user of > it, it should be safe to drop it entirely. Hmmm... I've try the dropping shrink_all_memory() today. but I've got bad result. In 3 times test, result were 2 times: kernel hang-up ;) 1 time: success, but make slower than with shrink_all_memory() about 100x times. Did you try to drop it yourself on your machine? Is this success? > > Please see following shrink_all_memory() code. it's pretty small. it only have > > few vmscan preparation. I don't think it is hard to maintainance. > > No, it's not, but I'm really not sure it's worth keeping. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: KOSAKI Motohiro on 3 Nov 2009 09:10 > Hi. > > KOSAKI Motohiro wrote: > >> I haven't given much thought to numa awareness in hibernate code, but I > >> can say that the shrink_all_memory interface is woefully inadequate as > >> far as zone awareness goes. Since lowmem needs to be atomically restored > >> before we can restore highmem, we really need to be able to ask for a > >> particular number of pages of a particular zone type to be freed. > > > > Honestly, I am not suspend/hibernation expert. Can I ask why caller need to know > > per-zone number of freed pages information? if hibernation don't need highmem. > > following incremental patch prevent highmem reclaim perfectly. Is it enough? > > (Disclaimer: I don't think about highmem a lot any more, and might have > forgotten some of the details, or swsusp's algorithms might have > changed. Rafael might need to correct some of this...) > > Imagine that you have a system with 1000 pages of lowmem and 5000 pages > of highmem. Of these, 950 lowmem pages are in use and 500 highmem pages > are in use. > > In order to to be able to save an image, we need to be able to do an > atomic copy of those lowmem pages. > > You might think that we could just copy everything into the spare > highmem pages, but we can't because mapping and unmapping the highmem > pages as we copy the data will leave us with an inconsistent copy. > Depending on the configuration, it might (for example) have one page - > say on in the pagetables - reflecting one page being kmapped and another > page - containing the variables that record what kmap slots are used, > for example - recording a different page being kmapped. > > What we do, then, is seek to atomically copy the lowmem pages to lowmem. > That requires, however, that we have at least half of the lowmem pages > free. So, then, we need a function that lets us free lowmem pages only. > > I hope that makes it clearer. > > > --- > > mm/vmscan.c | 2 +- > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > diff --git a/mm/vmscan.c b/mm/vmscan.c > > index e6ea011..7fb3435 100644 > > --- a/mm/vmscan.c > > +++ b/mm/vmscan.c > > @@ -2265,7 +2265,7 @@ unsigned long shrink_all_memory(unsigned long nr_to_reclaim) > > { > > struct reclaim_state reclaim_state; > > struct scan_control sc = { > > - .gfp_mask = GFP_HIGHUSER_MOVABLE, > > + .gfp_mask = GFP_KERNEL, > > .may_swap = 1, > > .may_unmap = 1, > > .may_writepage = 1, > > I don't think so. I think what we really need is: > > shrink_memory_type(gfp_mask, pages_needed) > > That is, a function that would let us say "Free 489 pages of lowmem" or > "Free 983 pages of highmem" or "Free 340 pages of any kind of memory". > (The later might be used if we just want to free some pages because the > image as it stands is too big for the storage available). I can add gfp_mask argument to shrink_all_memory(). it's easy. but I obviously need to help from PM folks for meaningful change. I'm not sure How should we calculate required free memory for hibernation. Sidenote, current reclaim logic can't do "Free 983 pages of highmem". but I doubt it's really necessary. I guess we only need "reclaim lowmem" and/or "reclaim any kind of memory". -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Rafael J. Wysocki on 3 Nov 2009 17:00 On Tuesday 03 November 2009, KOSAKI Motohiro wrote: > > On Monday 02 November 2009, KOSAKI Motohiro wrote: > > > > > Then, This patch changed shrink_all_memory() to only the wrapper function of > > > > > do_try_to_free_pages(). it bring good reviewability and debuggability, and solve > > > > > above problems. > > > > > > > > > > side note: Reclaim logic unificication makes two good side effect. > > > > > - Fix recursive reclaim bug on shrink_all_memory(). > > > > > it did forgot to use PF_MEMALLOC. it mean the system be able to stuck into deadlock. > > > > > - Now, shrink_all_memory() got lockdep awareness. it bring good debuggability. > > > > > > > > As I said previously, I don't really see a reason to keep shrink_all_memory(). > > > > > > > > Do you think that removing it will result in performance degradation? > > > > > > Hmm... > > > Probably, I misunderstood your mention. I thought you suggested to kill > > > all hibernation specific reclaim code. I did. It's no performance degression. > > > (At least, I didn't observe) > > > > > > But, if you hope to kill shrink_all_memory() function itsef, the short answer is, > > > it's impossible. > > > > > > Current VM reclaim code need some preparetion to caller, and there are existing in > > > both alloc_pages_slowpath() and try_to_free_pages(). We can't omit its preparation. > > > > Well, my grepping for 'shrink_all_memory' throughout the entire kernel source > > code seems to indicate that hibernate_preallocate_memory() is the only current > > user of it. I may be wrong, but I doubt it, unless some new users have been > > added since 2.6.31. > > > > In case I'm not wrong, it should be safe to drop it from > > hibernate_preallocate_memory(), because it's there for performance reasons > > only. Now, since hibernate_preallocate_memory() appears to be the only user of > > it, it should be safe to drop it entirely. > > Hmmm... > I've try the dropping shrink_all_memory() today. but I've got bad result. > > In 3 times test, result were > > 2 times: kernel hang-up ;) > 1 time: success, but make slower than with shrink_all_memory() about 100x times. > > > Did you try to drop it yourself on your machine? Is this success? Generally, yes, but the performance was hit really badly. So, the conclusion is that we need shrink_all_memory() for things to work, which is kind of interesting. In that case, please feel free to add Acked-by: Rafael J. Wysocki <rjw(a)sisk.pl> to the patch. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Rafael J. Wysocki on 3 Nov 2009 17:00
On Tuesday 03 November 2009, Nigel Cunningham wrote: > Hi Rafael. > > Rafael J. Wysocki wrote: > > On Monday 02 November 2009, Nigel Cunningham wrote: > >> Hi. > > > > Hi, > > > >> KOSAKI Motohiro wrote: > >>>> I haven't given much thought to numa awareness in hibernate code, but I > >>>> can say that the shrink_all_memory interface is woefully inadequate as > >>>> far as zone awareness goes. Since lowmem needs to be atomically restored > >>>> before we can restore highmem, we really need to be able to ask for a > >>>> particular number of pages of a particular zone type to be freed. > >>> Honestly, I am not suspend/hibernation expert. Can I ask why caller need to know > >>> per-zone number of freed pages information? if hibernation don't need highmem. > >>> following incremental patch prevent highmem reclaim perfectly. Is it enough? > >> (Disclaimer: I don't think about highmem a lot any more, and might have > >> forgotten some of the details, or swsusp's algorithms might have > >> changed. Rafael might need to correct some of this...) > >> > >> Imagine that you have a system with 1000 pages of lowmem and 5000 pages > >> of highmem. Of these, 950 lowmem pages are in use and 500 highmem pages > >> are in use. > >> > >> In order to to be able to save an image, we need to be able to do an > >> atomic copy of those lowmem pages. > >> > >> You might think that we could just copy everything into the spare > >> highmem pages, but we can't because mapping and unmapping the highmem > >> pages as we copy the data will leave us with an inconsistent copy. > > > > This isn't the case any more for the mainline hibernate code. We use highmem > > for storing image data as well as lowmem. > > Highmem for storing copies of lowmem pages? It is possible in theory, but I don't think it happens in practice given the way in which the memory is freed. Still copy_data_page() takes this possibility into account. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |