| Message ID | 1595681998-19193-4-git-send-email-alex.shi@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | per memcg lru lock | expand |
On Sat, Jul 25, 2020 at 6:00 AM Alex Shi <alex.shi@linux.alibaba.com> wrote: > > There is no compact_defer_limit. It should be compact_defer_shift in > use. and add compact_order_failed explanation. > > Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: linux-mm@kvack.org > Cc: linux-kernel@vger.kernel.org > --- > include/linux/mmzone.h | 1 + > mm/compaction.c | 2 +- > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h > index f6f884970511..14c668b7e793 100644 > --- a/include/linux/mmzone.h > +++ b/include/linux/mmzone.h > @@ -512,6 +512,7 @@ struct zone { > * On compaction failure, 1<<compact_defer_shift compactions > * are skipped before trying again. The number attempted since > * last failure is tracked with compact_considered. > + * compact_order_failed is the minimum compaction failed order. > */ > unsigned int compact_considered; > unsigned int compact_defer_shift; > diff --git a/mm/compaction.c b/mm/compaction.c > index 86375605faa9..cd1ef9e5e638 100644 > --- a/mm/compaction.c > +++ b/mm/compaction.c > @@ -136,7 +136,7 @@ void __ClearPageMovable(struct page *page) > > /* > * Compaction is deferred when compaction fails to result in a page > - * allocation success. 1 << compact_defer_limit compactions are skipped up > + * allocation success. compact_defer_shift++, compactions are skipped up > * to a limit of 1 << COMPACT_MAX_DEFER_SHIFT > */ > void defer_compaction(struct zone *zone, int order) So this doesn't read right. I wouldn't keep the "++," in the explanation, and if we are going to refer to a limit of "1 << COMPACT_MAX_DEFER_SHIFT" then maybe this should be left as "1 << compact_defer_shift".
>> * Compaction is deferred when compaction fails to result in a page >> - * allocation success. 1 << compact_defer_limit compactions are skipped up >> + * allocation success. compact_defer_shift++, compactions are skipped up >> * to a limit of 1 << COMPACT_MAX_DEFER_SHIFT >> */ >> void defer_compaction(struct zone *zone, int order) > > So this doesn't read right. I wouldn't keep the "++," in the > explanation, and if we are going to refer to a limit of "1 << > COMPACT_MAX_DEFER_SHIFT" then maybe this should be left as "1 << > compact_defer_shift". > Thanks for comments! So is the changed patch fine? -- From 80ffde4c8e13ba2ad1ad5175dbaef245c2fe49bc Mon Sep 17 00:00:00 2001 From: Alex Shi <alex.shi@linux.alibaba.com> Date: Tue, 26 May 2020 09:47:01 +0800 Subject: [PATCH] mm/compaction: correct the comments of compact_defer_shift There is no compact_defer_limit. It should be compact_defer_shift in use. and add compact_order_failed explanation. Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: linux-mm@kvack.org Cc: linux-kernel@vger.kernel.org --- include/linux/mmzone.h | 1 + mm/compaction.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h index f6f884970511..14c668b7e793 100644 --- a/include/linux/mmzone.h +++ b/include/linux/mmzone.h @@ -512,6 +512,7 @@ struct zone { * On compaction failure, 1<<compact_defer_shift compactions * are skipped before trying again. The number attempted since * last failure is tracked with compact_considered. + * compact_order_failed is the minimum compaction failed order. */ unsigned int compact_considered; unsigned int compact_defer_shift; diff --git a/mm/compaction.c b/mm/compaction.c index 86375605faa9..4950240cd455 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -136,7 +136,7 @@ void __ClearPageMovable(struct page *page) /* * Compaction is deferred when compaction fails to result in a page - * allocation success. 1 << compact_defer_limit compactions are skipped up + * allocation success. 1 << compact_defer_shift, compactions are skipped up * to a limit of 1 << COMPACT_MAX_DEFER_SHIFT */ void defer_compaction(struct zone *zone, int order)
On Tue, Jul 28, 2020 at 4:59 AM Alex Shi <alex.shi@linux.alibaba.com> wrote: > > > >> * Compaction is deferred when compaction fails to result in a page > >> - * allocation success. 1 << compact_defer_limit compactions are skipped up > >> + * allocation success. compact_defer_shift++, compactions are skipped up > >> * to a limit of 1 << COMPACT_MAX_DEFER_SHIFT > >> */ > >> void defer_compaction(struct zone *zone, int order) > > > > So this doesn't read right. I wouldn't keep the "++," in the > > explanation, and if we are going to refer to a limit of "1 << > > COMPACT_MAX_DEFER_SHIFT" then maybe this should be left as "1 << > > compact_defer_shift". > > > > Thanks for comments! So is the changed patch fine? > -- > > From 80ffde4c8e13ba2ad1ad5175dbaef245c2fe49bc Mon Sep 17 00:00:00 2001 > From: Alex Shi <alex.shi@linux.alibaba.com> > Date: Tue, 26 May 2020 09:47:01 +0800 > Subject: [PATCH] mm/compaction: correct the comments of compact_defer_shift > > There is no compact_defer_limit. It should be compact_defer_shift in > use. and add compact_order_failed explanation. > > Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: linux-mm@kvack.org > Cc: linux-kernel@vger.kernel.org > --- > include/linux/mmzone.h | 1 + > mm/compaction.c | 2 +- > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h > index f6f884970511..14c668b7e793 100644 > --- a/include/linux/mmzone.h > +++ b/include/linux/mmzone.h > @@ -512,6 +512,7 @@ struct zone { > * On compaction failure, 1<<compact_defer_shift compactions > * are skipped before trying again. The number attempted since > * last failure is tracked with compact_considered. > + * compact_order_failed is the minimum compaction failed order. > */ > unsigned int compact_considered; > unsigned int compact_defer_shift; > diff --git a/mm/compaction.c b/mm/compaction.c > index 86375605faa9..4950240cd455 100644 > --- a/mm/compaction.c > +++ b/mm/compaction.c > @@ -136,7 +136,7 @@ void __ClearPageMovable(struct page *page) > > /* > * Compaction is deferred when compaction fails to result in a page > - * allocation success. 1 << compact_defer_limit compactions are skipped up > + * allocation success. 1 << compact_defer_shift, compactions are skipped up > * to a limit of 1 << COMPACT_MAX_DEFER_SHIFT > */ > void defer_compaction(struct zone *zone, int order) Yes, that looks better to me. Reviewed-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h index f6f884970511..14c668b7e793 100644 --- a/include/linux/mmzone.h +++ b/include/linux/mmzone.h @@ -512,6 +512,7 @@ struct zone { * On compaction failure, 1<<compact_defer_shift compactions * are skipped before trying again. The number attempted since * last failure is tracked with compact_considered. + * compact_order_failed is the minimum compaction failed order. */ unsigned int compact_considered; unsigned int compact_defer_shift; diff --git a/mm/compaction.c b/mm/compaction.c index 86375605faa9..cd1ef9e5e638 100644 --- a/mm/compaction.c +++ b/mm/compaction.c @@ -136,7 +136,7 @@ void __ClearPageMovable(struct page *page) /* * Compaction is deferred when compaction fails to result in a page - * allocation success. 1 << compact_defer_limit compactions are skipped up + * allocation success. compact_defer_shift++, compactions are skipped up * to a limit of 1 << COMPACT_MAX_DEFER_SHIFT */ void defer_compaction(struct zone *zone, int order)
There is no compact_defer_limit. It should be compact_defer_shift in use. and add compact_order_failed explanation. Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: linux-mm@kvack.org Cc: linux-kernel@vger.kernel.org --- include/linux/mmzone.h | 1 + mm/compaction.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)