linux-kernel - Re: [PATCH 01/83] mm: Add kfd_process pointer to mm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Fri, 11 Jul 2014 11:42:32 -0400
From:	Jerome Glisse <j.glisse@...il.com>
To:	Oded Gabbay <oded.gabbay@...il.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	David Airlie <airlied@...ux.ie>,
	Alex Deucher <alexander.deucher@....com>,
	linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
	John Bridgman <John.Bridgman@....com>,
	Andrew Lewycky <Andrew.Lewycky@....com>,
	Joerg Roedel <joro@...tes.org>, linux-mm <linux-mm@...ck.org>,
	Oded Gabbay <oded.gabbay@....com>,
	Rik van Riel <riel@...hat.com>,
	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
	Mel Gorman <mgorman@...e.de>,
	Peter Zijlstra <peterz@...radead.org>,
	Joonsoo Kim <iamjoonsoo.kim@....com>,
	Jérôme Glisse <jglisse@...hat.com>,
	Michel Lespinasse <walken@...gle.com>
Subject: Re: [PATCH 01/83] mm: Add kfd_process pointer to mm_struct

On Fri, Jul 11, 2014 at 12:47:26AM +0300, Oded Gabbay wrote:
> This patch enables the KFD to retrieve the kfd_process
> object from the process's mm_struct. This is needed because kfd_process
> lifespan is bound to the process's mm_struct lifespan.
> 
> When KFD is notified about an mm_struct tear-down, it checks if the
> kfd_process pointer is valid. If so, it releases the kfd_process object
> and all relevant resources.
> 
> Signed-off-by: Oded Gabbay <oded.gabbay@....com>
> ---
>  include/linux/mm_types.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 678097c..6179107 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -20,6 +20,10 @@
>  struct hmm;
>  #endif
>  
> +#ifdef CONFIG_HSA_RADEON
> +struct kfd_process;
> +#endif
> +
>  #ifndef AT_VECTOR_SIZE_ARCH
>  #define AT_VECTOR_SIZE_ARCH 0
>  #endif
> @@ -439,6 +443,16 @@ struct mm_struct {
>  	 */
>  	struct hmm *hmm;
>  #endif
> +#if defined(CONFIG_HSA_RADEON) || defined(CONFIG_HSA_RADEON_MODULE)
> +	/*
> +	 * kfd always register an mmu_notifier we rely on mmu notifier to keep
> +	 * refcount on mm struct as well as forbiding registering kfd on a
> +	 * dying mm
> +	 *
> +	 * This field is set with mmap_sem old in write mode.
> +	 */
> +	struct kfd_process *kfd_process;
> +#endif

I understand the need to bind kfd to mm life time but this is wrong
on several level. First we do not want per driver define flag here.
Second this should be a IOMMU/PASID pointer of some sort, i am sure
that Intel will want to add itself too to mm_struct so instead of
having each IOMMU add a pointer here, i would rather see a generic
pointer to a generic IOMMU struct and have this use generic IOMMU
code that can then call specific user dispatch function.

I know this add a layer but this is not a critical code path and
should never be.

I am adding Jesse as he might have thought on that.

So this one is NAK

Cheers,
Jérôme

>  #if defined(CONFIG_TRANSPARENT_HUGEPAGE) && !USE_SPLIT_PMD_PTLOCKS
>  	pgtable_t pmd_huge_pte; /* protected by page_table_lock */
>  #endif
> -- 
> 1.9.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/