On Fri, Jan 28 2022 at 12:28, Fenghua Yu wrote: > To avoid complexity of updating each thread's PASID status (e.g. sending > IPI to update IA32_PASID MSR) on allocating and freeing PASID, once > allocated and assigned to an mm, the PASID stays with the mm for the > rest of the mm's lifetime. A reference to the PASID is taken on > allocating the PASID. Binding/unbinding the PASID won't change refcount. > The reference is dropped on mm exit and thus the PASID is freed. > > Two helpers mm_pasid_set() and mm_pasid_drop() are defined in mm because > the PASID operations handle the pasid member in mm_struct and should be > part of mm operations. Because IOASID's reference count is not used any > more and removed, unused ioasid_get() and iommu_sva_free_pasid() > are deleted and ioasid_put() is renamed to ioasid_free(). > > 20-bit PASID allows up to 1M processes bound to PASIDs at the same time. > With cgroups and other controls that might limit the number of process > creation, the limited number of PASIDs is not a realistic issue for > lazy PASID free.
Please take a step back and think hard about it whether that changelog makes sense to you a year from now. Let me walk you through: > To avoid complexity of updating each thread's PASID status (e.g. sending > IPI to update IA32_PASID MSR) on allocating and freeing PASID, once > allocated and assigned to an mm, the PASID stays with the mm for the > rest of the mm's lifetime. You are missing the oportunity to tell a story about the history of this decision here: PASIDs are process wide. It was attempted to use refcounted PASIDs to free them when the last thread drops the refcount. This turned out to be complex and error prone. Given the fact that the PASID space is 20 bits, which allows up to 1M processes to have a PASID associated concurrently, PASID resource exhaustion is not a realistic concern. Therefore it was decided to simplify the approach and stick with lazy on demand PASID allocation, but drop the eager free approach and make a allocated PASID lifetime bound to the life time of the process. > A reference to the PASID is taken on allocating the > PASID. Binding/unbinding the PASID won't change refcount. The > reference is dropped on mm exit and thus the PASID is freed. There is no refcount in play anymore, right? So how does this part of the changelog make any sense? This is followed by: > Two helpers mm_pasid_set() and mm_pasid_drop() are defined in mm because > the PASID operations handle the pasid member in mm_struct and should be > part of mm operations. Because IOASID's reference count is not used any > more and removed, unused ioasid_get() and iommu_sva_free_pasid() > are deleted and ioasid_put() is renamed to ioasid_free(). which does not provide much rationale and just blurbs about _what_ the patch is doing and not about the why and the connection to the above. And the refcount removal section contradicts the stale text above. So this paragraph should be something like this: Get rid of the refcounting mechanisms and replace/rename the interfaces to reflect this new approach. Hmm? Thanks, tglx _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu