Btw., another question that made me wonder already when doing the original patch: why is it that x86-64 properly uses locking for mm_pin_all(), yet i386 doesn''t need to? Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 16/3/07 13:47, "Jan Beulich" <jbeulich@novell.com> wrote:> Btw., another question that made me wonder already when doing the > original patch: why is it that x86-64 properly uses locking for mm_pin_all(), > yet i386 doesn''t need to?Neither needs to. Well, assuming you''re not using CONFIG_PREEMPT. :-) Either we are in stop_machine context, or we have offlined all other CPUs via cpu hotplug. In the absence of involuntary preemption it''s therefore safe to proceed without locking. But probably inadvisable (we''d like to support full CONFIG_PREEMPT sometime in the future)... I think the 386 code should be changed to match x86/64. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 16/3/07 14:13, "Keir Fraser" <keir@xensource.com> wrote:> Either we are in stop_machine context, or we have offlined all other CPUs > via cpu hotplug. In the absence of involuntary preemption it''s therefore > safe to proceed without locking. But probably inadvisable (we''d like to > support full CONFIG_PREEMPT sometime in the future)... I think the 386 code > should be changed to match x86/64.Actually it''s not so easy. We walk the pgd_list and so do not have the mm associated with the pgd. And in some cases there may not be an mm, if we walk the list after a pgd is allocated but before it is attached to an mm. So I think we should simply disable preemption in the i386 case. Which is done easily by simply taking the pgd_lock (which it would be polite to do anyway, since we''re walking the pgd_list). With preemption disabled we''re definitely safe because pinning is non-blocking and all other CPUs are safely spinning in stop_machine or have been offlined. Actually preemption is already disabled by the caller (for a different reason) but it''s not nice to rely on that. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>>> Keir Fraser <keir@xensource.com> 16.03.07 15:30 >>> >On 16/3/07 14:13, "Keir Fraser" <keir@xensource.com> wrote: > >> Either we are in stop_machine context, or we have offlined all other CPUs >> via cpu hotplug. In the absence of involuntary preemption it''s therefore >> safe to proceed without locking. But probably inadvisable (we''d like to >> support full CONFIG_PREEMPT sometime in the future)... I think the 386 code >> should be changed to match x86/64. > >Actually it''s not so easy. We walk the pgd_list and so do not have the mm >associated with the pgd. And in some cases there may not be an mm, if we >walk the list after a pgd is allocated but before it is attached to an mm.But without being attached to an mm, the user portion of the pgd should be blank? I really can''t follow why i386 requires so much more special handling here than x86-64.>So I think we should simply disable preemption in the i386 case. Which is >done easily by simply taking the pgd_lock (which it would be polite to do >anyway, since we''re walking the pgd_list). With preemption disabled we''re >definitely safe because pinning is non-blocking and all other CPUs are >safely spinning in stop_machine or have been offlined.And what about the pgd_test_and_unpin() case?>Actually preemption is already disabled by the caller (for a different >reason) but it''s not nice to rely on that.Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 16/3/07 15:15, "Jan Beulich" <jbeulich@novell.com> wrote:> But without being attached to an mm, the user portion of the pgd should > be blank? I really can''t follow why i386 requires so much more special > handling here than x86-64.PAE allocates the pmd''s early (in pgd_alloc() in fact). So any pgd_alloc()ed pgd must be captured by mm_pin_all().> And what about the pgd_test_and_unpin() case?It''s only called from pgd_alloc, pgd_free, pgd_dtor. I''m not sure it''s needed in all those places, but it''s needed in some of them to deal with the case that _arch_exit_mmap() didn''t unpin the pgd, because a PAE pgd does not lose its pmd''s until pgd_free(). In none of those cases is there a concurrency problem: there''s no mm associated with the pgd and hence no concurrency issue. I just checked in a patch to clarify some of this, particularly around mm_pin_all, as c/s 14408. Feel free to submit more clarification patches -- I suspect a few more comments around these functions would help avoid confusion! -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel