public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] intel: make sure VG_CLEAR() will always do memory clear
@ 2013-11-20  9:22 Zhenyu Wang
  2013-11-20 11:36 ` Damien Lespiau
  0 siblings, 1 reply; 5+ messages in thread
From: Zhenyu Wang @ 2013-11-20  9:22 UTC (permalink / raw)
  To: intel-gfx

If valgrind is not available, current VG_CLEAR() would just ignore
memory clear operation which might make invalid ioctl argument. So
make sure VG_CLEAR() will always clear memory.
---
 intel/intel_bufmgr_gem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index df6fcec..389f73a 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -74,7 +74,7 @@
 #define VG(x)
 #endif
 
-#define VG_CLEAR(s) VG(memset(&s, 0, sizeof(s)))
+#define VG_CLEAR(s) (memset(&s, 0, sizeof(s)))
 
 #define DBG(...) do {					\
 	if (bufmgr_gem->bufmgr.debug)			\
-- 
1.8.4.3

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] intel: make sure VG_CLEAR() will always do memory clear
  2013-11-20  9:22 [PATCH] intel: make sure VG_CLEAR() will always do memory clear Zhenyu Wang
@ 2013-11-20 11:36 ` Damien Lespiau
  2013-11-20 15:53   ` Zhenyu Wang
  0 siblings, 1 reply; 5+ messages in thread
From: Damien Lespiau @ 2013-11-20 11:36 UTC (permalink / raw)
  To: Zhenyu Wang; +Cc: intel-gfx

On Wed, Nov 20, 2013 at 05:22:48PM +0800, Zhenyu Wang wrote:
> If valgrind is not available, current VG_CLEAR() would just ignore
> memory clear operation which might make invalid ioctl argument. So
> make sure VG_CLEAR() will always clear memory.
> ---
>  intel/intel_bufmgr_gem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
> index df6fcec..389f73a 100644
> --- a/intel/intel_bufmgr_gem.c
> +++ b/intel/intel_bufmgr_gem.c
> @@ -74,7 +74,7 @@
>  #define VG(x)
>  #endif
>  
> -#define VG_CLEAR(s) VG(memset(&s, 0, sizeof(s)))
> +#define VG_CLEAR(s) (memset(&s, 0, sizeof(s)))

VG_CLEAR() is really just for valgrind. If you need to set some specific
variable/field to 0 then you need to set it to 0 and not rely on
VG_CLEAR() to do it for you.

What's the actual issue you have?

-- 
Damien

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] intel: make sure VG_CLEAR() will always do memory clear
  2013-11-20 11:36 ` Damien Lespiau
@ 2013-11-20 15:53   ` Zhenyu Wang
  2013-11-20 16:59     ` Damien Lespiau
  0 siblings, 1 reply; 5+ messages in thread
From: Zhenyu Wang @ 2013-11-20 15:53 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 1351 bytes --]

On 2013.11.20 11:36:20 +0000, Damien Lespiau wrote:
> On Wed, Nov 20, 2013 at 05:22:48PM +0800, Zhenyu Wang wrote:
> > If valgrind is not available, current VG_CLEAR() would just ignore
> > memory clear operation which might make invalid ioctl argument. So
> > make sure VG_CLEAR() will always clear memory.
> > ---
> >  intel/intel_bufmgr_gem.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
> > index df6fcec..389f73a 100644
> > --- a/intel/intel_bufmgr_gem.c
> > +++ b/intel/intel_bufmgr_gem.c
> > @@ -74,7 +74,7 @@
> >  #define VG(x)
> >  #endif
> >  
> > -#define VG_CLEAR(s) VG(memset(&s, 0, sizeof(s)))
> > +#define VG_CLEAR(s) (memset(&s, 0, sizeof(s)))
> 
> VG_CLEAR() is really just for valgrind. If you need to set some specific
> variable/field to 0 then you need to set it to 0 and not rely on
> VG_CLEAR() to do it for you.
> 

ok, in valgrind case it does memory clear for ioctl args which I think is
good behavior in non-valgrind case too.

> What's the actual issue you have?
> 

During testing on recent get reset status ioctl, if !HAVE_VALGRIND, stats
argument is not cleared, which failed in kernel check.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] intel: make sure VG_CLEAR() will always do memory clear
  2013-11-20 15:53   ` Zhenyu Wang
@ 2013-11-20 16:59     ` Damien Lespiau
  2013-11-21  2:16       ` Zhenyu Wang
  0 siblings, 1 reply; 5+ messages in thread
From: Damien Lespiau @ 2013-11-20 16:59 UTC (permalink / raw)
  To: Zhenyu Wang; +Cc: intel-gfx

On Wed, Nov 20, 2013 at 11:53:54PM +0800, Zhenyu Wang wrote:
> > VG_CLEAR() is really just for valgrind. If you need to set some specific
> > variable/field to 0 then you need to set it to 0 and not rely on
> > VG_CLEAR() to do it for you.
> > 
> 
> ok, in valgrind case it does memory clear for ioctl args which I think is
> good behavior in non-valgrind case too.

Ian just submitted a patch that memset the whole structure.

> > What's the actual issue you have?
> > 
> 
> During testing on recent get reset status ioctl, if !HAVE_VALGRIND, stats
> argument is not cleared, which failed in kernel check.

Right, so the fix is (was) to zero the fields checked by the kernel
explicitely, not change the VG() macro, which is just used in testing
(and it should this way).

HTH,

-- 
Damien

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] intel: make sure VG_CLEAR() will always do memory clear
  2013-11-20 16:59     ` Damien Lespiau
@ 2013-11-21  2:16       ` Zhenyu Wang
  0 siblings, 0 replies; 5+ messages in thread
From: Zhenyu Wang @ 2013-11-21  2:16 UTC (permalink / raw)
  To: Damien Lespiau; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 361 bytes --]

On 2013.11.20 16:59:22 +0000, Damien Lespiau wrote:
> Right, so the fix is (was) to zero the fields checked by the kernel
> explicitely, not change the VG() macro, which is just used in testing
> (and it should this way).
> 

That's fine. Thanks.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-11-21  2:18 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-20  9:22 [PATCH] intel: make sure VG_CLEAR() will always do memory clear Zhenyu Wang
2013-11-20 11:36 ` Damien Lespiau
2013-11-20 15:53   ` Zhenyu Wang
2013-11-20 16:59     ` Damien Lespiau
2013-11-21  2:16       ` Zhenyu Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox