Re: [PATCH 2/3] Refactor zone_reclaim (v2)
* MinChan Kim [2010-12-15 07:38:42]:
> On Tue, Dec 14, 2010 at 8:45 PM, Balbir Singh
> wrote:
> > * MinChan Kim [2010-12-14 19:01:26]:
> >
> >> Hi Balbir,
> >>
> >> On Fri, Dec 10, 2010 at 11:31 PM, Balbir Singh
> >> wrote:
> >> > Move reusable functionality outside of zone_reclaim.
> >> > Make zone_reclaim_unmapped_pages modular
> >> >
> >> > Signed-off-by: Balbir Singh
> >> > ---
> >> > mm/vmscan.c | 35 +++
> >> > 1 files changed, 23 insertions(+), 12 deletions(-)
> >> >
> >> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> >> > index e841cae..4e2ad05 100644
> >> > --- a/mm/vmscan.c
> >> > +++ b/mm/vmscan.c
> >> > @@ -2815,6 +2815,27 @@ static long zone_pagecache_reclaimable(struct
> >> > zone *zone)
> >> > }
> >> >
> >> > /*
> >> > + * Helper function to reclaim unmapped pages, we might add something
> >> > + * similar to this for slab cache as well. Currently this function
> >> > + * is shared with __zone_reclaim()
> >> > + */
> >> > +static inline void
> >> > +zone_reclaim_unmapped_pages(struct zone *zone, struct scan_control *sc,
> >> > + unsigned long nr_pages)
> >> > +{
> >> > + int priority;
> >> > + /*
> >> > + * Free memory by calling shrink zone with increasing
> >> > + * priorities until we have enough memory freed.
> >> > + */
> >> > + priority = ZONE_RECLAIM_PRIORITY;
> >> > + do {
> >> > + shrink_zone(priority, zone, sc);
> >> > + priority--;
> >> > + } while (priority >= 0 && sc->nr_reclaimed < nr_pages);
> >> > +}
> >>
> >> As I said previous version, zone_reclaim_unmapped_pages doesn't have
> >> any functions related to reclaim unmapped pages.
> >
> > The scan control point has the right arguments for implementing
> > reclaim of unmapped pages.
>
> I mean you should set up scan_control setup in this function.
> Current zone_reclaim_unmapped_pages doesn't have any specific routine
> related to reclaim unmapped pages.
> Otherwise, change the function name with just "zone_reclaim_pages". I
> think you don't want it.
Done, I renamed the function to zone_reclaim_pages.
Thanks!
--
Three Cheers,
Balbir
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Refactor zone_reclaim (v2)
* MinChan Kim [2010-12-14 19:01:26]:
> Hi Balbir,
>
> On Fri, Dec 10, 2010 at 11:31 PM, Balbir Singh
> wrote:
> > Move reusable functionality outside of zone_reclaim.
> > Make zone_reclaim_unmapped_pages modular
> >
> > Signed-off-by: Balbir Singh
> > ---
> > mm/vmscan.c | 35 +++
> > 1 files changed, 23 insertions(+), 12 deletions(-)
> >
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index e841cae..4e2ad05 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -2815,6 +2815,27 @@ static long zone_pagecache_reclaimable(struct zone
> > *zone)
> > }
> >
> > /*
> > + * Helper function to reclaim unmapped pages, we might add something
> > + * similar to this for slab cache as well. Currently this function
> > + * is shared with __zone_reclaim()
> > + */
> > +static inline void
> > +zone_reclaim_unmapped_pages(struct zone *zone, struct scan_control *sc,
> > + unsigned long nr_pages)
> > +{
> > + int priority;
> > + /*
> > + * Free memory by calling shrink zone with increasing
> > + * priorities until we have enough memory freed.
> > + */
> > + priority = ZONE_RECLAIM_PRIORITY;
> > + do {
> > + shrink_zone(priority, zone, sc);
> > + priority--;
> > + } while (priority >= 0 && sc->nr_reclaimed < nr_pages);
> > +}
>
> As I said previous version, zone_reclaim_unmapped_pages doesn't have
> any functions related to reclaim unmapped pages.
> The function name is rather strange.
> It would be better to add scan_control setup in function inner to
> reclaim only unmapped pages.
OK, that is an idea worth looking at, I'll revisit this function.
Thanks for the review!
--
Three Cheers,
Balbir
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Refactor zone_reclaim (v2)
On Tue, Dec 14, 2010 at 8:45 PM, Balbir Singh wrote:
> * MinChan Kim [2010-12-14 19:01:26]:
>
>> Hi Balbir,
>>
>> On Fri, Dec 10, 2010 at 11:31 PM, Balbir Singh
>> wrote:
>> > Move reusable functionality outside of zone_reclaim.
>> > Make zone_reclaim_unmapped_pages modular
>> >
>> > Signed-off-by: Balbir Singh
>> > ---
>> > mm/vmscan.c | 35 +++
>> > 1 files changed, 23 insertions(+), 12 deletions(-)
>> >
>> > diff --git a/mm/vmscan.c b/mm/vmscan.c
>> > index e841cae..4e2ad05 100644
>> > --- a/mm/vmscan.c
>> > +++ b/mm/vmscan.c
>> > @@ -2815,6 +2815,27 @@ static long zone_pagecache_reclaimable(struct zone
>> > *zone)
>> > }
>> >
>> > /*
>> > + * Helper function to reclaim unmapped pages, we might add something
>> > + * similar to this for slab cache as well. Currently this function
>> > + * is shared with __zone_reclaim()
>> > + */
>> > +static inline void
>> > +zone_reclaim_unmapped_pages(struct zone *zone, struct scan_control *sc,
>> > + unsigned long nr_pages)
>> > +{
>> > + int priority;
>> > + /*
>> > + * Free memory by calling shrink zone with increasing
>> > + * priorities until we have enough memory freed.
>> > + */
>> > + priority = ZONE_RECLAIM_PRIORITY;
>> > + do {
>> > + shrink_zone(priority, zone, sc);
>> > + priority--;
>> > + } while (priority >= 0 && sc->nr_reclaimed < nr_pages);
>> > +}
>>
>> As I said previous version, zone_reclaim_unmapped_pages doesn't have
>> any functions related to reclaim unmapped pages.
>
> The scan control point has the right arguments for implementing
> reclaim of unmapped pages.
I mean you should set up scan_control setup in this function.
Current zone_reclaim_unmapped_pages doesn't have any specific routine
related to reclaim unmapped pages.
Otherwise, change the function name with just "zone_reclaim_pages". I
think you don't want it.
--
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Refactor zone_reclaim (v2)
* MinChan Kim [2010-12-14 19:01:26]:
> Hi Balbir,
>
> On Fri, Dec 10, 2010 at 11:31 PM, Balbir Singh
> wrote:
> > Move reusable functionality outside of zone_reclaim.
> > Make zone_reclaim_unmapped_pages modular
> >
> > Signed-off-by: Balbir Singh
> > ---
> > mm/vmscan.c | 35 +++
> > 1 files changed, 23 insertions(+), 12 deletions(-)
> >
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index e841cae..4e2ad05 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -2815,6 +2815,27 @@ static long zone_pagecache_reclaimable(struct zone
> > *zone)
> > }
> >
> > /*
> > + * Helper function to reclaim unmapped pages, we might add something
> > + * similar to this for slab cache as well. Currently this function
> > + * is shared with __zone_reclaim()
> > + */
> > +static inline void
> > +zone_reclaim_unmapped_pages(struct zone *zone, struct scan_control *sc,
> > + unsigned long nr_pages)
> > +{
> > + int priority;
> > + /*
> > + * Free memory by calling shrink zone with increasing
> > + * priorities until we have enough memory freed.
> > + */
> > + priority = ZONE_RECLAIM_PRIORITY;
> > + do {
> > + shrink_zone(priority, zone, sc);
> > + priority--;
> > + } while (priority >= 0 && sc->nr_reclaimed < nr_pages);
> > +}
>
> As I said previous version, zone_reclaim_unmapped_pages doesn't have
> any functions related to reclaim unmapped pages.
The scan control point has the right arguments for implementing
reclaim of unmapped pages.
> The function name is rather strange.
> It would be better to add scan_control setup in function inner to
> reclaim only unmapped pages.
>
> --
> Kind regards,
> Minchan Kim
--
Three Cheers,
Balbir
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Refactor zone_reclaim (v2)
Hi Balbir,
On Fri, Dec 10, 2010 at 11:31 PM, Balbir Singh
wrote:
> Move reusable functionality outside of zone_reclaim.
> Make zone_reclaim_unmapped_pages modular
>
> Signed-off-by: Balbir Singh
> ---
> mm/vmscan.c | 35 +++
> 1 files changed, 23 insertions(+), 12 deletions(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index e841cae..4e2ad05 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2815,6 +2815,27 @@ static long zone_pagecache_reclaimable(struct zone
> *zone)
> }
>
> /*
> + * Helper function to reclaim unmapped pages, we might add something
> + * similar to this for slab cache as well. Currently this function
> + * is shared with __zone_reclaim()
> + */
> +static inline void
> +zone_reclaim_unmapped_pages(struct zone *zone, struct scan_control *sc,
> + unsigned long nr_pages)
> +{
> + int priority;
> + /*
> + * Free memory by calling shrink zone with increasing
> + * priorities until we have enough memory freed.
> + */
> + priority = ZONE_RECLAIM_PRIORITY;
> + do {
> + shrink_zone(priority, zone, sc);
> + priority--;
> + } while (priority >= 0 && sc->nr_reclaimed < nr_pages);
> +}
As I said previous version, zone_reclaim_unmapped_pages doesn't have
any functions related to reclaim unmapped pages.
The function name is rather strange.
It would be better to add scan_control setup in function inner to
reclaim only unmapped pages.
--
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] Refactor zone_reclaim (v2)
Move reusable functionality outside of zone_reclaim.
Make zone_reclaim_unmapped_pages modular
Signed-off-by: Balbir Singh
---
mm/vmscan.c | 35 +++
1 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/mm/vmscan.c b/mm/vmscan.c
index e841cae..4e2ad05 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2815,6 +2815,27 @@ static long zone_pagecache_reclaimable(struct zone *zone)
}
/*
+ * Helper function to reclaim unmapped pages, we might add something
+ * similar to this for slab cache as well. Currently this function
+ * is shared with __zone_reclaim()
+ */
+static inline void
+zone_reclaim_unmapped_pages(struct zone *zone, struct scan_control *sc,
+ unsigned long nr_pages)
+{
+ int priority;
+ /*
+* Free memory by calling shrink zone with increasing
+* priorities until we have enough memory freed.
+*/
+ priority = ZONE_RECLAIM_PRIORITY;
+ do {
+ shrink_zone(priority, zone, sc);
+ priority--;
+ } while (priority >= 0 && sc->nr_reclaimed < nr_pages);
+}
+
+/*
* Try to free up some pages from this zone through reclaim.
*/
static int __zone_reclaim(struct zone *zone, gfp_t gfp_mask, unsigned int
order)
@@ -2823,7 +2844,6 @@ static int __zone_reclaim(struct zone *zone, gfp_t
gfp_mask, unsigned int order)
const unsigned long nr_pages = 1 << order;
struct task_struct *p = current;
struct reclaim_state reclaim_state;
- int priority;
struct scan_control sc = {
.may_writepage = !!(zone_reclaim_mode & RECLAIM_WRITE),
.may_unmap = !!(zone_reclaim_mode & RECLAIM_SWAP),
@@ -2847,17 +2867,8 @@ static int __zone_reclaim(struct zone *zone, gfp_t
gfp_mask, unsigned int order)
reclaim_state.reclaimed_slab = 0;
p->reclaim_state = &reclaim_state;
- if (zone_pagecache_reclaimable(zone) > zone->min_unmapped_pages) {
- /*
-* Free memory by calling shrink zone with increasing
-* priorities until we have enough memory freed.
-*/
- priority = ZONE_RECLAIM_PRIORITY;
- do {
- shrink_zone(priority, zone, &sc);
- priority--;
- } while (priority >= 0 && sc.nr_reclaimed < nr_pages);
- }
+ if (zone_pagecache_reclaimable(zone) > zone->min_unmapped_pages)
+ zone_reclaim_unmapped_pages(zone, &sc, nr_pages);
nr_slab_pages0 = zone_page_state(zone, NR_SLAB_RECLAIMABLE);
if (nr_slab_pages0 > zone->min_slab_pages) {
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
