on 2010-3-4 11:22, Nick Piggin wrote: ... >> + /* >> + * After current->mems_allowed is set to a new value, current will >> + * allocate new pages for the migrating memory region. So we must >> + * ensure that update of current->mems_allowed have been completed >> + * by this moment. >> + */ >> + smp_wmb(); >> do_migrate_pages(mm, from, to, MPOL_MF_MOVE_ALL); >> >> guarantee_online_mems(task_cs(tsk),&tsk->mems_allowed); >> + >> + /* >> + * After doing migrate pages, current will allocate new pages for >> + * itself not the other tasks. So we must ensure that update of >> + * current->mems_allowed have been completed by this moment. >> + */ >> + smp_wmb(); > > The comments don't really make sense. A task always sees its own > memory operations in program order. You keep saying *current* allocates > pages so *current*->mems_allowed must be updated. This doesn't make > sense. Do you mean to say tsk->? > > Secondly, memory ordering operations do not ensure anything is > completed. They only ensure ordering. So to make sense to use them, > you generally need corresponding barriers in other code that can > run concurrently. > > So you need to comment what is being ordered (ie. at least 2 memory > operations). And what other code might be running that requires this > ordering. > > You need to comment to all these sites and operations. Sprinkling of > memory barriers just gets unmaintainable. My thought is wrong. I thought the kernel might call do_migrate_pages() before updating ->mems_allowed, so I used smp_wmb() to ensure this order. In fact, this problem which I worried can't occur, so these smp_wmb() is unnecessary. Thanks! Miao -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxxx For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>