subject:"\[PATCH 1\/1\] drm\/i915\:Do not use kfree\(\) to free kmem_cache

[PATCH 1/1] drm/i915: Do not use kfree() to free kmem_cache_alloc() return value

2018-04-05 Thread Xidong Wang

In eb_lookup_vmas(), the return value of kmem_cache_alloc() is freed
with kfree(). I think the expected paired function is kmem_cahce_free().

Signed-off-by: Xidong Wang 
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 8c170db..0414228 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -728,7 +728,7 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
 
err = radix_tree_insert(handles_vma, handle, vma);
if (unlikely(err)) {
-   kfree(lut);
+   kmem_cache_free(eb->i915->luts, lut);
goto err_obj;
}
 
-- 
2.7.4


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] drm/i915: Do not use kfree() to free kmem_cache_alloc() return value

2018-04-04 Thread Chris Wilson

Quoting Xidong Wang (2018-04-04 08:37:54)
> In eb_lookup_vmas(), the return value of kmem_cache_alloc() is freed
> with kfree(). I think the expected paired function is kmem_cahce_free().
> 
> Signed-off-by: Xidong Wang 

Thank you for the fix; applied.
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] drm/i915:Do not use kfree() to free kmem_cache_alloc() return value

2018-04-04 Thread kbuild test robot

Hi Xidong,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on v4.16 next-20180403]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Xidong-Wang/drm-i915-Do-not-use-kfree-to-free-kmem_cache_alloc-return-value/20180404-193341
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-x009-201813 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/gpu//drm/i915/i915_gem_execbuffer.c: In function 'eb_lookup_vmas':
>> drivers/gpu//drm/i915/i915_gem_execbuffer.c:731:20: error: passing argument 
>> 1 of 'kmem_cache_free' from incompatible pointer type 
>> [-Werror=incompatible-pointer-types]
   kmem_cache_free(lut);
   ^~~
   In file included from include/linux/reservation.h:44:0,
from drivers/gpu//drm/i915/i915_gem_execbuffer.c:30:
   include/linux/slab.h:356:6: note: expected 'struct kmem_cache *' but 
argument is of type 'struct i915_lut_handle *'
void kmem_cache_free(struct kmem_cache *, void *);
 ^~~
>> drivers/gpu//drm/i915/i915_gem_execbuffer.c:731:4: error: too few arguments 
>> to function 'kmem_cache_free'
   kmem_cache_free(lut);
   ^~~
   In file included from include/linux/reservation.h:44:0,
from drivers/gpu//drm/i915/i915_gem_execbuffer.c:30:
   include/linux/slab.h:356:6: note: declared here
void kmem_cache_free(struct kmem_cache *, void *);
 ^~~
   cc1: some warnings being treated as errors

vim +/kmem_cache_free +731 drivers/gpu//drm/i915/i915_gem_execbuffer.c

   685  
   686  static int eb_lookup_vmas(struct i915_execbuffer *eb)
   687  {
   688  struct radix_tree_root *handles_vma = >ctx->handles_vma;
   689  struct drm_i915_gem_object *obj;
   690  unsigned int i;
   691  int err;
   692  
   693  if (unlikely(i915_gem_context_is_closed(eb->ctx)))
   694  return -ENOENT;
   695  
   696  if (unlikely(i915_gem_context_is_banned(eb->ctx)))
   697  return -EIO;
   698  
   699  INIT_LIST_HEAD(>relocs);
   700  INIT_LIST_HEAD(>unbound);
   701  
   702  for (i = 0; i < eb->buffer_count; i++) {
   703  u32 handle = eb->exec[i].handle;
   704  struct i915_lut_handle *lut;
   705  struct i915_vma *vma;
   706  
   707  vma = radix_tree_lookup(handles_vma, handle);
   708  if (likely(vma))
   709  goto add_vma;
   710  
   711  obj = i915_gem_object_lookup(eb->file, handle);
   712  if (unlikely(!obj)) {
   713  err = -ENOENT;
   714  goto err_vma;
   715  }
   716  
   717  vma = i915_vma_instance(obj, eb->vm, NULL);
   718  if (unlikely(IS_ERR(vma))) {
   719  err = PTR_ERR(vma);
   720  goto err_obj;
   721  }
   722  
   723  lut = kmem_cache_alloc(eb->i915->luts, GFP_KERNEL);
   724  if (unlikely(!lut)) {
   725  err = -ENOMEM;
   726  goto err_obj;
   727  }
   728  
   729  err = radix_tree_insert(handles_vma, handle, vma);
   730  if (unlikely(err)) {
 > 731  kmem_cache_free(lut);
   732  goto err_obj;
   733  }
   734  
   735  /* transfer ref to ctx */
   736  vma->open_count++;
   737  list_add(>obj_link, >lut_list);
   738  list_add(>ctx_link, >ctx->handles_list);
   739  lut->ctx = eb->ctx;
   740  lut->handle = handle;
   741  
   742  add_vma:
   743  err = eb_add_vma(eb, i, vma);
   744  if (unlikely(err))
   745  goto err_vma;
   746  
   747  GEM_BUG_ON(vma != eb->vma[i]);
   748  GEM_BUG_ON(vma->exec_flags != >flags[i]);
   749  }
   750  
   751  /* take note of the batch buffer before we might reorder the 
lists */
   752  i = eb_batch_index(eb);
   753  eb->batch = eb->vma[i];
   754  GEM_BUG_ON(eb->batch->exec_flags != >flags[i]);
   755  
   756  /*
   757   * SNA is doing fancy tricks with compressing batch buffers, 
which leads
   758   * to negative relocation deltas. Usually that works out ok 
since the
   759   * relocate address is still positive, except

Re: [PATCH 1/1] drm/i915:Do not use kfree() to free kmem_cache_alloc() return value

2018-04-04 Thread kbuild test robot

Hi Xidong,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on v4.16 next-20180403]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Xidong-Wang/drm-i915-Do-not-use-kfree-to-free-kmem_cache_alloc-return-value/20180404-193341
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/gpu/drm/i915/i915_gem_execbuffer.c:731:41: sparse: incorrect type in 
>> argument 1 (different base types) @@expected struct kmem_cache 
>> * @@got struct i915_lut_struct kmem_cache * @@
   drivers/gpu/drm/i915/i915_gem_execbuffer.c:731:41:expected struct 
kmem_cache *
   drivers/gpu/drm/i915/i915_gem_execbuffer.c:731:41:got struct 
i915_lut_handle *[assigned] lut
>> drivers/gpu/drm/i915/i915_gem_execbuffer.c:731:40: sparse: not enough 
>> arguments for function kmem_cache_free
   drivers/gpu/drm/i915/i915_gem_execbuffer.c: In function 'eb_lookup_vmas':
   drivers/gpu/drm/i915/i915_gem_execbuffer.c:731:20: error: passing argument 1 
of 'kmem_cache_free' from incompatible pointer type 
[-Werror=incompatible-pointer-types]
   kmem_cache_free(lut);
   ^~~
   In file included from include/linux/reservation.h:44:0,
from drivers/gpu/drm/i915/i915_gem_execbuffer.c:30:
   include/linux/slab.h:356:6: note: expected 'struct kmem_cache *' but 
argument is of type 'struct i915_lut_handle *'
void kmem_cache_free(struct kmem_cache *, void *);
 ^~~
   drivers/gpu/drm/i915/i915_gem_execbuffer.c:731:4: error: too few arguments 
to function 'kmem_cache_free'
   kmem_cache_free(lut);
   ^~~
   In file included from include/linux/reservation.h:44:0,
from drivers/gpu/drm/i915/i915_gem_execbuffer.c:30:
   include/linux/slab.h:356:6: note: declared here
void kmem_cache_free(struct kmem_cache *, void *);
 ^~~
   cc1: some warnings being treated as errors

vim +731 drivers/gpu/drm/i915/i915_gem_execbuffer.c

   685  
   686  static int eb_lookup_vmas(struct i915_execbuffer *eb)
   687  {
   688  struct radix_tree_root *handles_vma = >ctx->handles_vma;
   689  struct drm_i915_gem_object *obj;
   690  unsigned int i;
   691  int err;
   692  
   693  if (unlikely(i915_gem_context_is_closed(eb->ctx)))
   694  return -ENOENT;
   695  
   696  if (unlikely(i915_gem_context_is_banned(eb->ctx)))
   697  return -EIO;
   698  
   699  INIT_LIST_HEAD(>relocs);
   700  INIT_LIST_HEAD(>unbound);
   701  
   702  for (i = 0; i < eb->buffer_count; i++) {
   703  u32 handle = eb->exec[i].handle;
   704  struct i915_lut_handle *lut;
   705  struct i915_vma *vma;
   706  
   707  vma = radix_tree_lookup(handles_vma, handle);
   708  if (likely(vma))
   709  goto add_vma;
   710  
   711  obj = i915_gem_object_lookup(eb->file, handle);
   712  if (unlikely(!obj)) {
   713  err = -ENOENT;
   714  goto err_vma;
   715  }
   716  
   717  vma = i915_vma_instance(obj, eb->vm, NULL);
   718  if (unlikely(IS_ERR(vma))) {
   719  err = PTR_ERR(vma);
   720  goto err_obj;
   721  }
   722  
   723  lut = kmem_cache_alloc(eb->i915->luts, GFP_KERNEL);
   724  if (unlikely(!lut)) {
   725  err = -ENOMEM;
   726  goto err_obj;
   727  }
   728  
   729  err = radix_tree_insert(handles_vma, handle, vma);
   730  if (unlikely(err)) {
 > 731  kmem_cache_free(lut);
   732  goto err_obj;
   733  }
   734  
   735  /* transfer ref to ctx */
   736  vma->open_count++;
   737  list_add(>obj_link, >lut_list);
   738  list_add(>ctx_link, >ctx->handles_list);
   739  lut->ctx = eb->ctx;
   740  lut->handle = handle;
   741  
   742  add_vma:
   743  err = eb_add_vma(eb, i, vma);
   744  if (unlikely(err))
   745  goto err_vma;
   746  
   747  GEM_BUG_ON(vma != eb->vma[i]);
   748  GEM_BUG_ON(vma->exec_flags != >flags[i]);
   749  }
   750  
   751  /* take note of the batch buffer before we might reorder

Re: [PATCH 1/1] drm/i915:Do not use kfree() to free kmem_cache_alloc() return value

2018-04-04 Thread kbuild test robot

Hi Xidong,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-intel/for-linux-next]
[also build test WARNING on v4.16 next-20180403]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Xidong-Wang/drm-i915-Do-not-use-kfree-to-free-kmem_cache_alloc-return-value/20180404-193341
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: i386-randconfig-a1-201813 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All warnings (new ones prefixed by >>):

   drivers/gpu//drm/i915/i915_gem_execbuffer.c: In function 'eb_lookup_vmas':
>> drivers/gpu//drm/i915/i915_gem_execbuffer.c:731:20: warning: passing 
>> argument 1 of 'kmem_cache_free' from incompatible pointer type
   kmem_cache_free(lut);
   ^
   In file included from include/linux/reservation.h:44:0,
from drivers/gpu//drm/i915/i915_gem_execbuffer.c:30:
   include/linux/slab.h:356:6: note: expected 'struct kmem_cache *' but 
argument is of type 'struct i915_lut_handle *'
void kmem_cache_free(struct kmem_cache *, void *);
 ^
   drivers/gpu//drm/i915/i915_gem_execbuffer.c:731:4: error: too few arguments 
to function 'kmem_cache_free'
   kmem_cache_free(lut);
   ^
   In file included from include/linux/reservation.h:44:0,
from drivers/gpu//drm/i915/i915_gem_execbuffer.c:30:
   include/linux/slab.h:356:6: note: declared here
void kmem_cache_free(struct kmem_cache *, void *);
 ^

vim +/kmem_cache_free +731 drivers/gpu//drm/i915/i915_gem_execbuffer.c

   685  
   686  static int eb_lookup_vmas(struct i915_execbuffer *eb)
   687  {
   688  struct radix_tree_root *handles_vma = >ctx->handles_vma;
   689  struct drm_i915_gem_object *obj;
   690  unsigned int i;
   691  int err;
   692  
   693  if (unlikely(i915_gem_context_is_closed(eb->ctx)))
   694  return -ENOENT;
   695  
   696  if (unlikely(i915_gem_context_is_banned(eb->ctx)))
   697  return -EIO;
   698  
   699  INIT_LIST_HEAD(>relocs);
   700  INIT_LIST_HEAD(>unbound);
   701  
   702  for (i = 0; i < eb->buffer_count; i++) {
   703  u32 handle = eb->exec[i].handle;
   704  struct i915_lut_handle *lut;
   705  struct i915_vma *vma;
   706  
   707  vma = radix_tree_lookup(handles_vma, handle);
   708  if (likely(vma))
   709  goto add_vma;
   710  
   711  obj = i915_gem_object_lookup(eb->file, handle);
   712  if (unlikely(!obj)) {
   713  err = -ENOENT;
   714  goto err_vma;
   715  }
   716  
   717  vma = i915_vma_instance(obj, eb->vm, NULL);
   718  if (unlikely(IS_ERR(vma))) {
   719  err = PTR_ERR(vma);
   720  goto err_obj;
   721  }
   722  
   723  lut = kmem_cache_alloc(eb->i915->luts, GFP_KERNEL);
   724  if (unlikely(!lut)) {
   725  err = -ENOMEM;
   726  goto err_obj;
   727  }
   728  
   729  err = radix_tree_insert(handles_vma, handle, vma);
   730  if (unlikely(err)) {
 > 731  kmem_cache_free(lut);
   732  goto err_obj;
   733  }
   734  
   735  /* transfer ref to ctx */
   736  vma->open_count++;
   737  list_add(>obj_link, >lut_list);
   738  list_add(>ctx_link, >ctx->handles_list);
   739  lut->ctx = eb->ctx;
   740  lut->handle = handle;
   741  
   742  add_vma:
   743  err = eb_add_vma(eb, i, vma);
   744  if (unlikely(err))
   745  goto err_vma;
   746  
   747  GEM_BUG_ON(vma != eb->vma[i]);
   748  GEM_BUG_ON(vma->exec_flags != >flags[i]);
   749  }
   750  
   751  /* take note of the batch buffer before we might reorder the 
lists */
   752  i = eb_batch_index(eb);
   753  eb->batch = eb->vma[i];
   754  GEM_BUG_ON(eb->batch->exec_flags != >flags[i]);
   755  
   756  /*
   757   * SNA is doing fancy tricks with compressing batch buffers, 
which leads
   758   * to negative relocation deltas. Usually that works out ok 
since the
   759   * relocate address is still positive, except when the batch is 
placed
   760   * very low in the GTT. Ensure this doesn't happen.
   761   *
   762

Re: [PATCH 1/1] drm/i915:Do not use kfree() to free kmem_cache_alloc() return value

2018-04-04 Thread Jani Nikula

On Wed, 04 Apr 2018, Xidong Wang  wrote:
> In eb_lookup_vmas(), lut, the return value of kmem_cache_alloc(), is freed
> with kfree().I think the expected paired function is kmem_cache_free().

Agreed. But did you try to compile your patch before sending?

Fixes: d1b48c1e7184 ("drm/i915: Replace execbuf vma ht with an idr")
Cc: Chris Wilson 
Cc: Tvrtko Ursulin 
Cc:  # v4.14+

BR,
Jani.

>
> Signed-off-by: Xidong Wang 
> ---
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 8c170db..08fe476 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -728,7 +728,7 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
>  
>   err = radix_tree_insert(handles_vma, handle, vma);
>   if (unlikely(err)) {
> - kfree(lut);
> + kmem_cache_free(lut);
>   goto err_obj;
>   }

-- 
Jani Nikula, Intel Open Source Technology Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] drm/i915: Do not use kfree() to free kmem_cache_alloc() return value

2018-04-04 Thread Chris Wilson

Quoting Xidong Wang (2018-04-04 08:37:54)
> In eb_lookup_vmas(), the return value of kmem_cache_alloc() is freed
> with kfree(). I think the expected paired function is kmem_cahce_free().
> 
> Signed-off-by: Xidong Wang 

That is indeed what it should be doing,

Fixes: d1b48c1e7184 ("drm/i915: Replace execbuf vma ht with an idr")
Cc: Chris Wilson 
Cc: Tvrtko Ursulin 
Cc:  # v4.14+
Reviewed-by: Chris Wilson 
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/1] drm/i915:Do not use kfree() to free kmem_cache_alloc() return value

2018-04-04 Thread Xidong Wang

In eb_lookup_vmas(), lut, the return value of kmem_cache_alloc(), is freed
with kfree().I think the expected paired function is kmem_cache_free().

Signed-off-by: Xidong Wang 
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 8c170db..08fe476 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -728,7 +728,7 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
 
err = radix_tree_insert(handles_vma, handle, vma);
if (unlikely(err)) {
-   kfree(lut);
+   kmem_cache_free(lut);
goto err_obj;
}
 
-- 
2.7.4


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/1] drm/i915: Do not use kfree() to free kmem_cache_alloc() return value

Re: [PATCH 1/1] drm/i915: Do not use kfree() to free kmem_cache_alloc() return value

Re: [PATCH 1/1] drm/i915:Do not use kfree() to free kmem_cache_alloc() return value

Re: [PATCH 1/1] drm/i915:Do not use kfree() to free kmem_cache_alloc() return value

Re: [PATCH 1/1] drm/i915:Do not use kfree() to free kmem_cache_alloc() return value

Re: [PATCH 1/1] drm/i915:Do not use kfree() to free kmem_cache_alloc() return value

Re: [PATCH 1/1] drm/i915: Do not use kfree() to free kmem_cache_alloc() return value

[PATCH 1/1] drm/i915:Do not use kfree() to free kmem_cache_alloc() return value

8 matches

Site Navigation

Mail list logo

Footer information