On Fri, Jan 21, 2005 at 05:27:11PM -0400, Mauricio Lin wrote: > Hi Andrea, > > I applied your patch and I am checking your code. It is really a very > interesting work. I have a question about the function > __set_current_state(TASK_INTERRUPTIBLE) you put in out_of_memory > function. Do not you think it would be better put set_current_state > instead of __set_current_state function? AFAIK the set_current_state > function is more feasible for SMP systems, right?
set_current_state is needed only when you need to place a memory barrier after __set_current_state. So it's needed in the usual wait_event loop, right after registering in the waitqueue. Example: unsigned long flags; wait->flags &= ~WQ_FLAG_EXCLUSIVE; spin_lock_irqsave(&q->lock, flags); if (list_empty(&wait->task_list)) __add_wait_queue(q, wait); /* * don't alter the task state if this is just going to * queue an async wait queue callback */ if (is_sync_wait(wait)) set_current_state(state); spin_unlock_irqrestore(&q->lock, flags); and even in the above is needed only because spin_unlock has inclusive semantics in ia64. In 2.4 there was no unlock at all after set_current_state and it was like this: set_current_state(TASK_UNINTERRUPTIBLE); \ if (condition) \ break; \ schedule(); \ The rule of thumb is that if there's nothing between set_current_state and schedule() then __set_current_state is more efficient and equally safe to use. And the oom killer path I posted falls in this category, nothing in between set_current_state and schedule, so no reason to place memory barries in there. Hope this helps ;) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/