[lkml]   [2005]   [Feb]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [RFC 2.6.11-rc2-mm2 7/7] mm: manual page migration -- sys_page_migrate
Minor comments ... nothing profound.

Ray wrote:
> once we agree on what the authority model should be.

Are the usual kill-like permissions sufficient?
You can migrate the pages of a process if you can kill it.


In the following routine, tighten up some vertical spacing,
add { ... } , ...

The migrated and count manipulations are confusing my
feeble brain. Is this thing supposed to return 0 if all
count pages are migrated? Sure seems that it does, as it
returns 'migrated', which is 'count - migrated', but that
migrated is really count, so it returns 'count - count',
which is zero. Huh ... The phrase 'return migrated' would
make me think it returned some count of how many were
migrated on success, not zero.

The variable name 'remains' is rather elaborate for what
looks like a trivial return case. But perhaps it actually
provides a better clue to the return value, which apparently
is the number of pages _not_ migrated successfully.

Think carefully about what each variable represents, and
then use each variable consistently.

And try to avoid the embedded 'return remains'. A function
header comment, saying what this routine does and returns might
be helpful.

static int
migrate_vma_common(struct list_head *page_list, short *node_map, int count)
int pass, remains, migrated;
struct page *page;

for (pass = 0; pass < 10; msleep(10), pass++) {
remains = try_to_migrate_pages(page_list, node_map);
if (remains < 0)
return remains;

migrated = 0;
if (!list_empty(page_list)) {
list_for_each_entry(page, page_list, lru)
} else {
migrated = count;
migrated = count - migrated;
return migrated;

Better init tmp_new_nodes, node_map to 0, or if tmp_old_news fails to
allocate, you might try freeing bogus values for the other two in

+ short *tmp_old_nodes;
+ short *tmp_new_nodes;
+ short *node_map;
+ ...
+ tmp_old_nodes = (short *) kmalloc(size, GFP_KERNEL);
+ tmp_new_nodes = (short *) kmalloc(size, GFP_KERNEL);
+ node_map = (short *) kmalloc(MAX_NUMNODES*sizeof(short), GFP_KERNEL);

I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <> 1.650.933.1373, 1.925.600.0401
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at
Please read the FAQ at

 \ /
  Last update: 2005-03-22 14:10    [W:0.204 / U:2.116 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site