diff for duplicates of <20110124113458.GX2232@cmpxchg.org> diff --git a/a/1.txt b/N1/1.txt index 77e42d4..6e49e12 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -10,59 +10,59 @@ On Mon, Jan 24, 2011 at 08:14:22PM +0900, Hiroyuki Kamezawa wrote: > >> > > A fix for 987eba66e0e6aa654d60881a14731a353ee0acb4 > >> > > > >> > > A clean up for mem_cgroup_move_parent(). -> >> > > - remove unnecessary initialization of local variable. -> >> > > - rename charge_size -> page_size -> >> > > - remove unnecessary (wrong) comment. +> >> > > - remove unnecessary initialization of local variable. +> >> > > - rename charge_size -> page_size +> >> > > - remove unnecessary (wrong) comment. > >> > > > >> > > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > >> > > --- -> >> > > mm/memcontrol.c | 17 +++++++++-------- -> >> > > 1 file changed, 9 insertions(+), 8 deletions(-) +> >> > > mm/memcontrol.c | 17 +++++++++-------- +> >> > > 1 file changed, 9 insertions(+), 8 deletions(-) > >> > > > >> > > Index: mmotm-0107/mm/memcontrol.c > >> > > =================================================================== > >> > > --- mmotm-0107.orig/mm/memcontrol.c > >> > > +++ mmotm-0107/mm/memcontrol.c > >> > > @@ -2265,7 +2265,7 @@ static int mem_cgroup_move_parent(struct -> >> > > struct cgroup *cg = child->css.cgroup; -> >> > > struct cgroup *pcg = cg->parent; -> >> > > struct mem_cgroup *parent; +> >> > > struct cgroup *cg = child->css.cgroup; +> >> > > struct cgroup *pcg = cg->parent; +> >> > > struct mem_cgroup *parent; > >> > > - int charge = PAGE_SIZE; > >> > > + int page_size; -> >> > > unsigned long flags; -> >> > > int ret; +> >> > > unsigned long flags; +> >> > > int ret; > >> > > > >> > > @@ -2278,22 +2278,23 @@ static int mem_cgroup_move_parent(struct -> >> > > goto out; -> >> > > if (isolate_lru_page(page)) -> >> > > goto put; +> >> > > goto out; +> >> > > if (isolate_lru_page(page)) +> >> > > goto put; > >> > > - /* The page is isolated from LRU and we have no race with splitting */ > >> > > - charge = PAGE_SIZE << compound_order(page); > >> > > + > >> > > + page_size = PAGE_SIZE << compound_order(page); > >> > > >> > Okay, so you remove the wrong comment, but that does not make the code -> >> > right. What protects compound_order from reading garbage because the +> >> > right. What protects compound_order from reading garbage because the > >> > page is currently splitting? > >> > > >> > >> == > >> static int mem_cgroup_move_account(struct page_cgroup *pc, -> >> struct mem_cgroup *from, struct mem_cgroup *to, -> >> bool uncharge, int charge_size) +> >> struct mem_cgroup *from, struct mem_cgroup *to, +> >> bool uncharge, int charge_size) > >> { -> >> int ret = -EINVAL; -> >> unsigned long flags; +> >> int ret = -EINVAL; +> >> unsigned long flags; > >> -> >> if ((charge_size > PAGE_SIZE) && !PageTransHuge(pc->page)) -> >> return -EBUSY; +> >> if ((charge_size > PAGE_SIZE) && !PageTransHuge(pc->page)) +> >> return -EBUSY; > >> == > >> > >> This is called under compound_lock(). Then, if someone breaks THP, > >> -EBUSY and retry. > > > > This charge_size contains exactly the garbage you just read from an -> > unprotected compound_order(). It could be anything if the page is +> > unprotected compound_order(). It could be anything if the page is > > split concurrently. > > Then, my recent fix to LRU accounting which use compound_order() is racy, too ? @@ -80,3 +80,10 @@ lock as well. Yes, I think this should work. This gives a sane size for try_charge and we still catch a split under the compound_lock later in move_account as you described above. + +-- +To unsubscribe, send a message with 'unsubscribe linux-mm' in +the body to majordomo@kvack.org. For more info on Linux MM, +see: http://www.linux-mm.org/ . +Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/ +Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> diff --git a/a/content_digest b/N1/content_digest index 7aa80e3..2f855e3 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -28,59 +28,59 @@ "> >> > > A fix for 987eba66e0e6aa654d60881a14731a353ee0acb4\n" "> >> > >\n" "> >> > > A clean up for mem_cgroup_move_parent().\n" - "> >> > > \302\240- remove unnecessary initialization of local variable.\n" - "> >> > > \302\240- rename charge_size -> page_size\n" - "> >> > > \302\240- remove unnecessary (wrong) comment.\n" + "> >> > > - remove unnecessary initialization of local variable.\n" + "> >> > > - rename charge_size -> page_size\n" + "> >> > > - remove unnecessary (wrong) comment.\n" "> >> > >\n" "> >> > > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>\n" "> >> > > ---\n" - "> >> > > \302\240mm/memcontrol.c | \302\240 17 +++++++++--------\n" - "> >> > > \302\2401 file changed, 9 insertions(+), 8 deletions(-)\n" + "> >> > > mm/memcontrol.c | 17 +++++++++--------\n" + "> >> > > 1 file changed, 9 insertions(+), 8 deletions(-)\n" "> >> > >\n" "> >> > > Index: mmotm-0107/mm/memcontrol.c\n" "> >> > > ===================================================================\n" "> >> > > --- mmotm-0107.orig/mm/memcontrol.c\n" "> >> > > +++ mmotm-0107/mm/memcontrol.c\n" "> >> > > @@ -2265,7 +2265,7 @@ static int mem_cgroup_move_parent(struct\n" - "> >> > > \302\240 struct cgroup *cg = child->css.cgroup;\n" - "> >> > > \302\240 struct cgroup *pcg = cg->parent;\n" - "> >> > > \302\240 struct mem_cgroup *parent;\n" + "> >> > > struct cgroup *cg = child->css.cgroup;\n" + "> >> > > struct cgroup *pcg = cg->parent;\n" + "> >> > > struct mem_cgroup *parent;\n" "> >> > > - int charge = PAGE_SIZE;\n" "> >> > > + int page_size;\n" - "> >> > > \302\240 unsigned long flags;\n" - "> >> > > \302\240 int ret;\n" + "> >> > > unsigned long flags;\n" + "> >> > > int ret;\n" "> >> > >\n" "> >> > > @@ -2278,22 +2278,23 @@ static int mem_cgroup_move_parent(struct\n" - "> >> > > \302\240 \302\240 \302\240 \302\240 \302\240 goto out;\n" - "> >> > > \302\240 if (isolate_lru_page(page))\n" - "> >> > > \302\240 \302\240 \302\240 \302\240 \302\240 goto put;\n" + "> >> > > goto out;\n" + "> >> > > if (isolate_lru_page(page))\n" + "> >> > > goto put;\n" "> >> > > - /* The page is isolated from LRU and we have no race with splitting */\n" "> >> > > - charge = PAGE_SIZE << compound_order(page);\n" "> >> > > +\n" "> >> > > + page_size = PAGE_SIZE << compound_order(page);\n" "> >> >\n" "> >> > Okay, so you remove the wrong comment, but that does not make the code\n" - "> >> > right. \302\240What protects compound_order from reading garbage because the\n" + "> >> > right. What protects compound_order from reading garbage because the\n" "> >> > page is currently splitting?\n" "> >> >\n" "> >>\n" "> >> ==\n" "> >> static int mem_cgroup_move_account(struct page_cgroup *pc,\n" - "> >> \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 struct mem_cgroup *from, struct mem_cgroup *to,\n" - "> >> \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 bool uncharge, int charge_size)\n" + "> >> struct mem_cgroup *from, struct mem_cgroup *to,\n" + "> >> bool uncharge, int charge_size)\n" "> >> {\n" - "> >> \302\240 \302\240 \302\240 \302\240 int ret = -EINVAL;\n" - "> >> \302\240 \302\240 \302\240 \302\240 unsigned long flags;\n" + "> >> int ret = -EINVAL;\n" + "> >> unsigned long flags;\n" "> >>\n" - "> >> \302\240 \302\240 \302\240 \302\240 if ((charge_size > PAGE_SIZE) && !PageTransHuge(pc->page))\n" - "> >> \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 \302\240 return -EBUSY;\n" + "> >> if ((charge_size > PAGE_SIZE) && !PageTransHuge(pc->page))\n" + "> >> return -EBUSY;\n" "> >> ==\n" "> >>\n" "> >> This is called under compound_lock(). Then, if someone breaks THP,\n" "> >> -EBUSY and retry.\n" "> >\n" "> > This charge_size contains exactly the garbage you just read from an\n" - "> > unprotected compound_order(). \302\240It could be anything if the page is\n" + "> > unprotected compound_order(). It could be anything if the page is\n" "> > split concurrently.\n" "> \n" "> Then, my recent fix to LRU accounting which use compound_order() is racy, too ?\n" @@ -97,6 +97,13 @@ "\n" "Yes, I think this should work. This gives a sane size for try_charge\n" "and we still catch a split under the compound_lock later in\n" - move_account as you described above. + "move_account as you described above.\n" + "\n" + "--\n" + "To unsubscribe, send a message with 'unsubscribe linux-mm' in\n" + "the body to majordomo@kvack.org. For more info on Linux MM,\n" + "see: http://www.linux-mm.org/ .\n" + "Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/\n" + "Don't email: <a href=mailto:\"dont@kvack.org\"> email@kvack.org </a>" -8d7a0047d2eed6cd544d345d19ab41fa7606032383ccd8d0505d7a4d55f54024 +0f4a130fe6b89f223f8ebfc182bfc3c04e6d8a390f37c5e7c6c23dbb1644407d
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.