On 02/23/2016 09:00 PM, Boqun Feng wrote:
Hi Waiman, On Tue, Feb 23, 2016 at 02:04:30PM -0500, Waiman Long wrote:} + +/* + * List selection is based on the CPU being used when the pcpu_list_add() + * function is called. However, deletion may be done by a different CPU. + * So we still need to use a lock to protect the content of the list. + */ +void pcpu_list_add(struct pcpu_list_node *node, struct pcpu_list_head *head) +{ + spinlock_t *lock; + + /* + * There is a very slight chance the cpu will be changed + * (by preemption) before calling spin_lock(). We only need to put + * the node in one of the per-cpu lists. It may not need to be + * that of the current cpu. + */Just curious about the comment here, what if the following happens: CPU 0 CPU 1 ===================== ===================== task_1: lock = this_cpu_ptr(&head->lock); // head->lock is on CPU0 <preempted> continue to task_1: spin_lock(lock); node->lockptr = lock; // head->list is on CPU1 list_add(&node->list, this_cpu_ptr(&head->list)); spin_unlock(lock); , which ends up the node is in the list on CPU1 while ->lockptr pointing to the lock on CPU0. If there is another node whose ->lockptr points to the lock on CPU1 and the node is in list on CPU1, what will happen if these two nodes get deleted simultaneously? Regards, Boqun
Yes, you are right. I should have acquired the per-cpu head pointer first and used it onward instead of accessing the lock and list in 2 separate operations. I will fix that in the next update.
Thanks for finding that. Cheers, Longman -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
