Prev: evdev keyboard driver stopped working after upgrading from Kernel 2.6.33-rc8 to 2.6.33 (official release)
Next: ACPI / EC: Remove race between EC driver and suspend process (rev. 3)
From: Miao Xie on 4 Mar 2010 04:40 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 from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |