On Wed, Oct 17, 2012 at 9:06 PM, Kamezawa Hiroyuki <kamezawa.hir...@jp.fujitsu.com> wrote: > if (vma && vma != priv->tail_vma) { > struct mm_struct *mm = vma->vm_mm; > +#ifdef CONFIG_NUMA > + task_lock(priv->task); > + __mpol_put(priv->task->mempolicy); > + task_unlock(priv->task); > +#endif > up_read(&mm->mmap_sem); > mmput(mm);
Please don't put #ifdef's inside code. It makes things really ugly and hard to read. And that is *especially* true in this case, since there's a pattern to all these things: > +#ifdef CONFIG_NUMA > + task_lock(priv->task); > + mpol_get(priv->task->mempolicy); > + task_unlock(priv->task); > +#endif > +#ifdef CONFIG_NUMA > + task_lock(priv->task); > + __mpol_put(priv->task->mempolicy); > + task_unlock(priv->task); > +#endif it really sounds like what you want to do is to just abstract a "numa_policy_get/put(priv)" operation. So you could make it be something like #ifdef CONFIG_NUMA static inline numa_policy_get(struct proc_maps_private *priv) { task_lock(priv->task); mpol_get(priv->task->mempolicy); task_unlock(priv->task); } .. same for the "put" function .. #else #define numa_policy_get(priv) do { } while (0) #define numa_policy_put(priv) do { } while (0) #endif and then you wouldn't have to have the #ifdef's in the middle of code, and I think it will be more readable in general. Sure, it is going to be a few more actual lines of patch, but there's no duplicated code sequence, and the added lines are just the syntax that makes it look better. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/