On Sun, 2005-08-07 at 16:20 -0700, Martin J. Bligh wrote: > Starting on the work to merge xen cleanly as a subarch. > Introduce make_pages_readonly and make_pages_writable where appropriate > for Xen, defined as a no-op on other subarches. Same for > add_context_to_unpinned and del_context_from_unpinned. > Abstract out install_ldt_entry() ... > cpumask_t mask; > preempt_disable(); > + make_pages_readonly(pc->ldt, (pc->size * LDT_ENTRY_SIZE) / > + PAGE_SIZE); > load_LDT(pc); > mask = cpumask_of_cpu(smp_processor_id()); > if (!cpus_equal(current->mm->cpu_vm_mask, mask)) > smp_call_function(flush_ldt, NULL, 1, 1); > preempt_enable(); > #else > + make_pages_readonly(pc->ldt, (pc->size * LDT_ENTRY_SIZE) / > + PAGE_SIZE);
You do that (size * LDT_ENTRY_SIZE / PAGE_SIZE) operation an awfully large number of times. Could you consider introducing a little helper, say ldt_size_pages()? Or, could you have a helper like make_ldt_readonly()? You don't have to export it, just use it in that one file. > This will do have no effect whatsover on platforms other than xen. ... > + memset(&mm->context, 0, sizeof(mm->context)); > init_MUTEX(&mm->context.sem); > - mm->context.size = 0; Could you please explain what this is for? It doesn't appear to be part of the abstraction. Every call path I can see to init_new_context() is immediately preceded by mm_alloc(), which memsets the entire mm. The context is a direct member of mm_struct, and should be zeroed along with the mm_alloc() memset. So, this seems a bit superfluous. In any future patches that you might post, please do one thing per patch, it makes them much easier to audit. -- Dave - 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/
