On Thu, Sep 20, 2012 at 11:41:11AM -0400, Johannes Weiner wrote: > On Thu, Sep 20, 2012 at 08:51:56AM +0900, Minchan Kim wrote: > > From: Minchan Kim <minchan@xxxxxxxxxx> > > Date: Thu, 20 Sep 2012 08:39:52 +0900 > > Subject: [PATCH] mm: revert 0def08e3, mm/mempolicy.c: check return code of > > check_range > > > > This patch reverts 0def08e3 because check_range can't fail in > > migrate_to_node with considering current usecases. > > > > Quote from Johannes > > " > > I think it makes sense to revert. Not because of the semantics, but I > > just don't see how check_range() could even fail for this callsite: > > > > 1. we pass mm->mmap->vm_start in there, so we should not fail due to > > find_vma() > > > > 2. we pass MPOL_MF_DISCONTIG_OK, so the discontig checks do not apply > > and so can not fail > > > > 3. we pass MPOL_MF_MOVE | MPOL_MF_MOVE_ALL, the page table loops will > > continue until addr == end, so we never fail with -EIO > > " > > > > And I add new VM_BUG_ON for checking migrate_to_node's future usecase > > which might pass to MPOL_MF_STRICT. > > > > Cc: KOSAKI Motohiro <kosaki.motohiro@xxxxxxxxxxxxxx> > > Cc: Mel Gorman <mgorman@xxxxxxx> > > Cc: Christoph Lameter <cl@xxxxxxxxx> > > Cc: David Rientjes <rientjes@xxxxxxxxxx> > > Cc: Vasiliy Kulikov <segooon@xxxxxxxxx> > > Suggested-by: Johannes Weiner <hannes@xxxxxxxxxxx> > > Signed-off-by: Minchan Kim <minchan@xxxxxxxxxx> > > --- > > mm/mempolicy.c | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > > index 3d64b36..9ec87bd 100644 > > --- a/mm/mempolicy.c > > +++ b/mm/mempolicy.c > > @@ -946,15 +946,16 @@ static int migrate_to_node(struct mm_struct *mm, int source, int dest, > > nodemask_t nmask; > > LIST_HEAD(pagelist); > > int err = 0; > > - struct vm_area_struct *vma; > > > > nodes_clear(nmask); > > node_set(source, nmask); > > > > - vma = check_range(mm, mm->mmap->vm_start, mm->task_size, &nmask, > > + /* > > + * Collect migrate pages and it shoudn't be failed. > > + */ > > + VM_BUG_ON(flags & MPOL_MF_STRICT); > > Adding a check and a comment is a good idea, but I'm not a big fan of > checking for MPOL_MF_STRICT in particular because it's one of the > invalid inputs, and so you need to extend this check when somebody > extends the spectrum of invalid inputs. I would much prefer checking > directly for !(flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) instead, which > would also make the possible inputs apparent without having to chase > up the call chain to find out what is usually passed in. > > And how about > > /* > * This does not "check" the range but isolates all pages that > * need migration. Between passing in the full user address > * space range and MPOL_MF_DISCONTIG_OK, this call can not fail. > */ > > ? Good idea. Thanks Hannes, >From 13ae84083ac1f057eafbeae49efba862aa7a470e Mon Sep 17 00:00:00 2001 From: Minchan Kim <minchan@xxxxxxxxxx> Date: Fri, 21 Sep 2012 08:17:37 +0900 Subject: [PATCH] mm: enhance comment and bug check This patch updates comment and bug check. It can be fold into [1]. [1] mm-revert-0def08e3-mm-mempolicyc-check-return-code-of-check_range.patch Cc: KOSAKI Motohiro <kosaki.motohiro@xxxxxxxxxxxxxx> Suggested-by: Johannes Weiner <hannes@xxxxxxxxxxx> Signed-off-by: Minchan Kim <minchan@xxxxxxxxxx> --- mm/mempolicy.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/mm/mempolicy.c b/mm/mempolicy.c index 9ec87bd..0b78fb9 100644 --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -951,9 +951,11 @@ static int migrate_to_node(struct mm_struct *mm, int source, int dest, node_set(source, nmask); /* - * Collect migrate pages and it shoudn't be failed. + * This does not "check" the range but isolates all pages that + * need migration. Between passing in the full user address + * space range and MPOL_MF_DISCONTIG_OK, this call can not fail. */ - VM_BUG_ON(flags & MPOL_MF_STRICT); + VM_BUG_ON(!(flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL))); check_range(mm, mm->mmap->vm_start, mm->task_size, &nmask, flags | MPOL_MF_DISCONTIG_OK, &pagelist); -- 1.7.9.5 -- Kind regards, Minchan Kim -- 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>