Re: [PATCH v2 2/3] drm/etnaviv: allocate unique ID per drm_file

2022-11-16 Thread Tvrtko Ursulin



On 16/11/2022 13:18, Philipp Zabel wrote:

On Fri, Sep 16, 2022 at 05:12:04PM +0200, Lucas Stach wrote:

Allows to easily track if several fd are pointing to the same
execution context due to being dup'ed.

Signed-off-by: Lucas Stach 
---
  drivers/gpu/drm/etnaviv/etnaviv_drv.c | 3 +++
  drivers/gpu/drm/etnaviv/etnaviv_drv.h | 1 +
  2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c 
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 1d2b4fb4bcf8..b69edb40ae2a 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -49,6 +49,7 @@ static void load_gpu(struct drm_device *dev)
  static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
  {
struct etnaviv_drm_private *priv = dev->dev_private;
+   static atomic_t ident = ATOMIC_INIT(0);
struct etnaviv_file_private *ctx;
int ret, i;
  
@@ -56,6 +57,8 @@ static int etnaviv_open(struct drm_device *dev, struct drm_file *file)

if (!ctx)
return -ENOMEM;
  
+	ctx->id = atomic_inc_return();


I suppose we can ignore that this could theoretically wrap around.


Depends on your usecases - if you can envisage a long running client, 
say the compositor, while other clients come and go then eventually 
these will not be unique and will break the fdinfo spec. Hence I used a 
cyclic xarray in i915 (aka idr). I would recommend you just do that and 
remove future debug sessions around the area of "why does gputop show 
nonsense all of a sudden".


Regards,

Tvrtko



Reviewed-by: Philipp Zabel 

regards
Philipp


Re: [PATCH v2 2/3] drm/etnaviv: allocate unique ID per drm_file

2022-11-16 Thread Philipp Zabel
On Fri, Sep 16, 2022 at 05:12:04PM +0200, Lucas Stach wrote:
> Allows to easily track if several fd are pointing to the same
> execution context due to being dup'ed.
> 
> Signed-off-by: Lucas Stach 
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c | 3 +++
>  drivers/gpu/drm/etnaviv/etnaviv_drv.h | 1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c 
> b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 1d2b4fb4bcf8..b69edb40ae2a 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -49,6 +49,7 @@ static void load_gpu(struct drm_device *dev)
>  static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
>  {
>   struct etnaviv_drm_private *priv = dev->dev_private;
> + static atomic_t ident = ATOMIC_INIT(0);
>   struct etnaviv_file_private *ctx;
>   int ret, i;
>  
> @@ -56,6 +57,8 @@ static int etnaviv_open(struct drm_device *dev, struct 
> drm_file *file)
>   if (!ctx)
>   return -ENOMEM;
>  
> + ctx->id = atomic_inc_return();

I suppose we can ignore that this could theoretically wrap around.

Reviewed-by: Philipp Zabel 

regards
Philipp


Re: [PATCH v2 2/3] drm/etnaviv: allocate unique ID per drm_file

2022-11-16 Thread Lucas Stach
Am Freitag, dem 16.09.2022 um 17:12 +0200 schrieb Lucas Stach:
> Allows to easily track if several fd are pointing to the same
> execution context due to being dup'ed.
> 
I would appreciate it if someone could review path 2 and 3 of this
series. I would like to include it in my next upstream PR.

Regards,
Lucas

> Signed-off-by: Lucas Stach 
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c | 3 +++
>  drivers/gpu/drm/etnaviv/etnaviv_drv.h | 1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c 
> b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 1d2b4fb4bcf8..b69edb40ae2a 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -49,6 +49,7 @@ static void load_gpu(struct drm_device *dev)
>  static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
>  {
>   struct etnaviv_drm_private *priv = dev->dev_private;
> + static atomic_t ident = ATOMIC_INIT(0);
>   struct etnaviv_file_private *ctx;
>   int ret, i;
>  
> @@ -56,6 +57,8 @@ static int etnaviv_open(struct drm_device *dev, struct 
> drm_file *file)
>   if (!ctx)
>   return -ENOMEM;
>  
> + ctx->id = atomic_inc_return();
> +
>   ctx->mmu = etnaviv_iommu_context_init(priv->mmu_global,
> priv->cmdbuf_suballoc);
>   if (!ctx->mmu) {
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h 
> b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> index f32f4771dada..851b4b4ef146 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> @@ -27,6 +27,7 @@ struct etnaviv_iommu_global;
>  #define ETNAVIV_SOFTPIN_START_ADDRESSSZ_4M /* must be >= 
> SUBALLOC_SIZE */
>  
>  struct etnaviv_file_private {
> + int id;
>   struct etnaviv_iommu_context*mmu;
>   struct drm_sched_entity sched_entity[ETNA_MAX_PIPES];
>  };




[PATCH v2 2/3] drm/etnaviv: allocate unique ID per drm_file

2022-09-16 Thread Lucas Stach
Allows to easily track if several fd are pointing to the same
execution context due to being dup'ed.

Signed-off-by: Lucas Stach 
---
 drivers/gpu/drm/etnaviv/etnaviv_drv.c | 3 +++
 drivers/gpu/drm/etnaviv/etnaviv_drv.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c 
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 1d2b4fb4bcf8..b69edb40ae2a 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -49,6 +49,7 @@ static void load_gpu(struct drm_device *dev)
 static int etnaviv_open(struct drm_device *dev, struct drm_file *file)
 {
struct etnaviv_drm_private *priv = dev->dev_private;
+   static atomic_t ident = ATOMIC_INIT(0);
struct etnaviv_file_private *ctx;
int ret, i;
 
@@ -56,6 +57,8 @@ static int etnaviv_open(struct drm_device *dev, struct 
drm_file *file)
if (!ctx)
return -ENOMEM;
 
+   ctx->id = atomic_inc_return();
+
ctx->mmu = etnaviv_iommu_context_init(priv->mmu_global,
  priv->cmdbuf_suballoc);
if (!ctx->mmu) {
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h 
b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
index f32f4771dada..851b4b4ef146 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
@@ -27,6 +27,7 @@ struct etnaviv_iommu_global;
 #define ETNAVIV_SOFTPIN_START_ADDRESS  SZ_4M /* must be >= SUBALLOC_SIZE */
 
 struct etnaviv_file_private {
+   int id;
struct etnaviv_iommu_context*mmu;
struct drm_sched_entity sched_entity[ETNA_MAX_PIPES];
 };
-- 
2.30.2