linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Make is_signed_type() simpler
@ 2019-09-29 20:06 Alexey Dobriyan
  2019-09-29 20:15 ` Steven Rostedt
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Alexey Dobriyan @ 2019-09-29 20:06 UTC (permalink / raw)
  To: akpm
  Cc: linux-kernel, jani.nikula, joonas.lahtinen, rodrigo.vivi,
	intel-gfx, rostedt, mingo, linux

* Simply compare -1 with 0,
* Drop unnecessary parenthesis sets

New macro leaves pointer as "unsigned type" but gives a warning,
which should be fine because asking whether a pointer is signed is
strange question.

I'm not sure what's going on in the i915 driver, it is shipping kernel
pointers to userspace.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 drivers/gpu/drm/i915/i915_trace.h |   86 +++++++++++++++++++-------------------
 include/linux/overflow.h          |    2 
 include/linux/trace_events.h      |    2 
 tools/include/linux/overflow.h    |    2 
 4 files changed, 46 insertions(+), 46 deletions(-)

--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -419,16 +419,16 @@ TRACE_EVENT(i915_gem_object_create,
 	    TP_ARGS(obj),
 
 	    TP_STRUCT__entry(
-			     __field(struct drm_i915_gem_object *, obj)
+			     __field(unsigned long, obj)
 			     __field(u64, size)
 			     ),
 
 	    TP_fast_assign(
-			   __entry->obj = obj;
+			   __entry->obj = (unsigned long)obj;
 			   __entry->size = obj->base.size;
 			   ),
 
-	    TP_printk("obj=%p, size=0x%llx", __entry->obj, __entry->size)
+	    TP_printk("obj=%p, size=0x%llx", (void *)__entry->obj, __entry->size)
 );
 
 TRACE_EVENT(i915_gem_shrink,
@@ -456,25 +456,25 @@ TRACE_EVENT(i915_vma_bind,
 	    TP_ARGS(vma, flags),
 
 	    TP_STRUCT__entry(
-			     __field(struct drm_i915_gem_object *, obj)
-			     __field(struct i915_address_space *, vm)
+			     __field(unsigned long, obj)
+			     __field(unsigned long, vm)
 			     __field(u64, offset)
 			     __field(u64, size)
 			     __field(unsigned, flags)
 			     ),
 
 	    TP_fast_assign(
-			   __entry->obj = vma->obj;
-			   __entry->vm = vma->vm;
+			   __entry->obj = (unsigned long)vma->obj;
+			   __entry->vm = (unsigned long)vma->vm;
 			   __entry->offset = vma->node.start;
 			   __entry->size = vma->node.size;
 			   __entry->flags = flags;
 			   ),
 
 	    TP_printk("obj=%p, offset=0x%016llx size=0x%llx%s vm=%p",
-		      __entry->obj, __entry->offset, __entry->size,
+		      (void *)__entry->obj, __entry->offset, __entry->size,
 		      __entry->flags & PIN_MAPPABLE ? ", mappable" : "",
-		      __entry->vm)
+		      (void *)__entry->vm)
 );
 
 TRACE_EVENT(i915_vma_unbind,
@@ -482,21 +482,21 @@ TRACE_EVENT(i915_vma_unbind,
 	    TP_ARGS(vma),
 
 	    TP_STRUCT__entry(
-			     __field(struct drm_i915_gem_object *, obj)
-			     __field(struct i915_address_space *, vm)
+			     __field(unsigned long, obj)
+			     __field(unsigned long, vm)
 			     __field(u64, offset)
 			     __field(u64, size)
 			     ),
 
 	    TP_fast_assign(
-			   __entry->obj = vma->obj;
-			   __entry->vm = vma->vm;
+			   __entry->obj = (unsigned long)vma->obj;
+			   __entry->vm = (unsigned long)vma->vm;
 			   __entry->offset = vma->node.start;
 			   __entry->size = vma->node.size;
 			   ),
 
 	    TP_printk("obj=%p, offset=0x%016llx size=0x%llx vm=%p",
-		      __entry->obj, __entry->offset, __entry->size, __entry->vm)
+		      (void *)__entry->obj, __entry->offset, __entry->size, (void *)__entry->vm)
 );
 
 TRACE_EVENT(i915_gem_object_pwrite,
@@ -504,19 +504,19 @@ TRACE_EVENT(i915_gem_object_pwrite,
 	    TP_ARGS(obj, offset, len),
 
 	    TP_STRUCT__entry(
-			     __field(struct drm_i915_gem_object *, obj)
+			     __field(unsigned long, obj)
 			     __field(u64, offset)
 			     __field(u64, len)
 			     ),
 
 	    TP_fast_assign(
-			   __entry->obj = obj;
+			   __entry->obj = (unsigned long)obj;
 			   __entry->offset = offset;
 			   __entry->len = len;
 			   ),
 
 	    TP_printk("obj=%p, offset=0x%llx, len=0x%llx",
-		      __entry->obj, __entry->offset, __entry->len)
+		      (void *)__entry->obj, __entry->offset, __entry->len)
 );
 
 TRACE_EVENT(i915_gem_object_pread,
@@ -524,19 +524,19 @@ TRACE_EVENT(i915_gem_object_pread,
 	    TP_ARGS(obj, offset, len),
 
 	    TP_STRUCT__entry(
-			     __field(struct drm_i915_gem_object *, obj)
+			     __field(unsigned long, obj)
 			     __field(u64, offset)
 			     __field(u64, len)
 			     ),
 
 	    TP_fast_assign(
-			   __entry->obj = obj;
+			   __entry->obj = (unsigned long)obj;
 			   __entry->offset = offset;
 			   __entry->len = len;
 			   ),
 
 	    TP_printk("obj=%p, offset=0x%llx, len=0x%llx",
-		      __entry->obj, __entry->offset, __entry->len)
+		      (void *)__entry->obj, __entry->offset, __entry->len)
 );
 
 TRACE_EVENT(i915_gem_object_fault,
@@ -544,21 +544,21 @@ TRACE_EVENT(i915_gem_object_fault,
 	    TP_ARGS(obj, index, gtt, write),
 
 	    TP_STRUCT__entry(
-			     __field(struct drm_i915_gem_object *, obj)
+			     __field(unsigned long, obj)
 			     __field(u64, index)
 			     __field(bool, gtt)
 			     __field(bool, write)
 			     ),
 
 	    TP_fast_assign(
-			   __entry->obj = obj;
+			   __entry->obj = (unsigned long)obj;
 			   __entry->index = index;
 			   __entry->gtt = gtt;
 			   __entry->write = write;
 			   ),
 
 	    TP_printk("obj=%p, %s index=%llu %s",
-		      __entry->obj,
+		      (void *)__entry->obj,
 		      __entry->gtt ? "GTT" : "CPU",
 		      __entry->index,
 		      __entry->write ? ", writable" : "")
@@ -569,14 +569,14 @@ DECLARE_EVENT_CLASS(i915_gem_object,
 	    TP_ARGS(obj),
 
 	    TP_STRUCT__entry(
-			     __field(struct drm_i915_gem_object *, obj)
+			     __field(unsigned long, obj)
 			     ),
 
 	    TP_fast_assign(
-			   __entry->obj = obj;
+			   __entry->obj = (unsigned long)obj;
 			   ),
 
-	    TP_printk("obj=%p", __entry->obj)
+	    TP_printk("obj=%p", (void *)__entry->obj)
 );
 
 DEFINE_EVENT(i915_gem_object, i915_gem_object_clflush,
@@ -595,7 +595,7 @@ TRACE_EVENT(i915_gem_evict,
 
 	    TP_STRUCT__entry(
 			     __field(u32, dev)
-			     __field(struct i915_address_space *, vm)
+			     __field(unsigned long, vm)
 			     __field(u64, size)
 			     __field(u64, align)
 			     __field(unsigned int, flags)
@@ -603,14 +603,14 @@ TRACE_EVENT(i915_gem_evict,
 
 	    TP_fast_assign(
 			   __entry->dev = vm->i915->drm.primary->index;
-			   __entry->vm = vm;
+			   __entry->vm = (unsigned long)vm;
 			   __entry->size = size;
 			   __entry->align = align;
 			   __entry->flags = flags;
 			  ),
 
 	    TP_printk("dev=%d, vm=%p, size=0x%llx, align=0x%llx %s",
-		      __entry->dev, __entry->vm, __entry->size, __entry->align,
+		      __entry->dev, (void *)__entry->vm, __entry->size, __entry->align,
 		      __entry->flags & PIN_MAPPABLE ? ", mappable" : "")
 );
 
@@ -620,7 +620,7 @@ TRACE_EVENT(i915_gem_evict_node,
 
 	    TP_STRUCT__entry(
 			     __field(u32, dev)
-			     __field(struct i915_address_space *, vm)
+			     __field(unsigned long, vm)
 			     __field(u64, start)
 			     __field(u64, size)
 			     __field(unsigned long, color)
@@ -629,7 +629,7 @@ TRACE_EVENT(i915_gem_evict_node,
 
 	    TP_fast_assign(
 			   __entry->dev = vm->i915->drm.primary->index;
-			   __entry->vm = vm;
+			   __entry->vm = (unsigned long)vm;
 			   __entry->start = node->start;
 			   __entry->size = node->size;
 			   __entry->color = node->color;
@@ -637,7 +637,7 @@ TRACE_EVENT(i915_gem_evict_node,
 			  ),
 
 	    TP_printk("dev=%d, vm=%p, start=0x%llx size=0x%llx, color=0x%lx, flags=%x",
-		      __entry->dev, __entry->vm,
+		      __entry->dev, (void *)__entry->vm,
 		      __entry->start, __entry->size,
 		      __entry->color, __entry->flags)
 );
@@ -648,15 +648,15 @@ TRACE_EVENT(i915_gem_evict_vm,
 
 	    TP_STRUCT__entry(
 			     __field(u32, dev)
-			     __field(struct i915_address_space *, vm)
+			     __field(unsigned long, vm)
 			    ),
 
 	    TP_fast_assign(
 			   __entry->dev = vm->i915->drm.primary->index;
-			   __entry->vm = vm;
+			   __entry->vm = (unsigned long)vm;
 			  ),
 
-	    TP_printk("dev=%d, vm=%p", __entry->dev, __entry->vm)
+	    TP_printk("dev=%d, vm=%p", __entry->dev, (void *)__entry->vm)
 );
 
 TRACE_EVENT(i915_request_queue,
@@ -922,16 +922,16 @@ DECLARE_EVENT_CLASS(i915_ppgtt,
 	TP_ARGS(vm),
 
 	TP_STRUCT__entry(
-			__field(struct i915_address_space *, vm)
+			__field(unsigned long, vm)
 			__field(u32, dev)
 	),
 
 	TP_fast_assign(
-			__entry->vm = vm;
+			__entry->vm = (unsigned long)vm;
 			__entry->dev = vm->i915->drm.primary->index;
 	),
 
-	TP_printk("dev=%u, vm=%p", __entry->dev, __entry->vm)
+	TP_printk("dev=%u, vm=%p", __entry->dev, (void *)__entry->vm)
 )
 
 DEFINE_EVENT(i915_ppgtt, i915_ppgtt_create,
@@ -957,20 +957,20 @@ DECLARE_EVENT_CLASS(i915_context,
 
 	TP_STRUCT__entry(
 			__field(u32, dev)
-			__field(struct i915_gem_context *, ctx)
+			__field(unsigned long, ctx)
 			__field(u32, hw_id)
-			__field(struct i915_address_space *, vm)
+			__field(unsigned long, vm)
 	),
 
 	TP_fast_assign(
 			__entry->dev = ctx->i915->drm.primary->index;
-			__entry->ctx = ctx;
+			__entry->ctx = (unsigned long)ctx;
 			__entry->hw_id = ctx->hw_id;
-			__entry->vm = ctx->vm;
+			__entry->vm = (unsigned long)ctx->vm;
 	),
 
 	TP_printk("dev=%u, ctx=%p, ctx_vm=%p, hw_id=%u",
-		  __entry->dev, __entry->ctx, __entry->vm, __entry->hw_id)
+		  __entry->dev, (void *)__entry->ctx, (void *)__entry->vm, __entry->hw_id)
 )
 
 DEFINE_EVENT(i915_context, i915_context_create,
--- a/include/linux/overflow.h
+++ b/include/linux/overflow.h
@@ -31,7 +31,7 @@
  * https://mail-index.netbsd.org/tech-misc/2007/02/05/0000.html -
  * credit to Christian Biere.
  */
-#define is_signed_type(type)       (((type)(-1)) < (type)1)
+#define is_signed_type(type)       ((type)-1 < 0)
 #define __type_half_max(type) ((type)1 << (8*sizeof(type) - 1 - is_signed_type(type)))
 #define type_max(T) ((T)((__type_half_max(T) - 1) + __type_half_max(T)))
 #define type_min(T) ((T)((T)-type_max(T)-(T)1))
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -546,7 +546,7 @@ extern int trace_add_event_call(struct trace_event_call *call);
 extern int trace_remove_event_call(struct trace_event_call *call);
 extern int trace_event_get_offsets(struct trace_event_call *call);
 
-#define is_signed_type(type)	(((type)(-1)) < (type)1)
+#define is_signed_type(type)	((type)-1 < 0)
 
 int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set);
 int trace_set_clr_event(const char *system, const char *event, int set);
--- a/tools/include/linux/overflow.h
+++ b/tools/include/linux/overflow.h
@@ -31,7 +31,7 @@
  * https://mail-index.netbsd.org/tech-misc/2007/02/05/0000.html -
  * credit to Christian Biere.
  */
-#define is_signed_type(type)       (((type)(-1)) < (type)1)
+#define is_signed_type(type)       ((type)-1 < 0)
 #define __type_half_max(type) ((type)1 << (8*sizeof(type) - 1 - is_signed_type(type)))
 #define type_max(T) ((T)((__type_half_max(T) - 1) + __type_half_max(T)))
 #define type_min(T) ((T)((T)-type_max(T)-(T)1))

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

* Re: [PATCH] Make is_signed_type() simpler
  2019-09-29 20:06 [PATCH] Make is_signed_type() simpler Alexey Dobriyan
@ 2019-09-29 20:15 ` Steven Rostedt
  2019-09-29 20:44   ` Alexey Dobriyan
  2019-09-29 20:21 ` Rasmus Villemoes
  2019-09-30 18:34 ` [Intel-gfx] " Ruhl, Michael J
  2 siblings, 1 reply; 8+ messages in thread
From: Steven Rostedt @ 2019-09-29 20:15 UTC (permalink / raw)
  To: Alexey Dobriyan
  Cc: akpm, linux-kernel, jani.nikula, joonas.lahtinen, rodrigo.vivi,
	intel-gfx, mingo, linux

On Sun, 29 Sep 2019 23:06:19 +0300
Alexey Dobriyan <adobriyan@gmail.com> wrote:

> * Simply compare -1 with 0,
> * Drop unnecessary parenthesis sets
> 
> New macro leaves pointer as "unsigned type" but gives a warning,
> which should be fine because asking whether a pointer is signed is
> strange question.
> 
> I'm not sure what's going on in the i915 driver, it is shipping kernel
> pointers to userspace.

This tells us what the patch does, not why.

-- Steve

> 
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>

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

* Re: [PATCH] Make is_signed_type() simpler
  2019-09-29 20:06 [PATCH] Make is_signed_type() simpler Alexey Dobriyan
  2019-09-29 20:15 ` Steven Rostedt
@ 2019-09-29 20:21 ` Rasmus Villemoes
  2019-09-29 21:09   ` Alexey Dobriyan
  2019-09-30 18:34 ` [Intel-gfx] " Ruhl, Michael J
  2 siblings, 1 reply; 8+ messages in thread
From: Rasmus Villemoes @ 2019-09-29 20:21 UTC (permalink / raw)
  To: Alexey Dobriyan, akpm
  Cc: linux-kernel, jani.nikula, joonas.lahtinen, rodrigo.vivi,
	intel-gfx, rostedt, mingo

On 29/09/2019 22.06, Alexey Dobriyan wrote:
> * Simply compare -1 with 0,
> * Drop unnecessary parenthesis sets
> 
> -#define is_signed_type(type)       (((type)(-1)) < (type)1)
> +#define is_signed_type(type)       ((type)-1 < 0)

NAK. I wrote it that way to avoid -Wtautological-compare when type is
unsigned.

Rasmus

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

* Re: [PATCH] Make is_signed_type() simpler
  2019-09-29 20:15 ` Steven Rostedt
@ 2019-09-29 20:44   ` Alexey Dobriyan
  2019-09-29 21:16     ` Steven Rostedt
  0 siblings, 1 reply; 8+ messages in thread
From: Alexey Dobriyan @ 2019-09-29 20:44 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: akpm, linux-kernel, jani.nikula, joonas.lahtinen, rodrigo.vivi,
	intel-gfx, mingo, linux

On Sun, Sep 29, 2019 at 04:15:31PM -0400, Steven Rostedt wrote:
> On Sun, 29 Sep 2019 23:06:19 +0300
> Alexey Dobriyan <adobriyan@gmail.com> wrote:
> 
> > * Simply compare -1 with 0,
> > * Drop unnecessary parenthesis sets
> > 
> > New macro leaves pointer as "unsigned type" but gives a warning,
> > which should be fine because asking whether a pointer is signed is
> > strange question.
> > 
> > I'm not sure what's going on in the i915 driver, it is shipping kernel
> > pointers to userspace.
> 
> This tells us what the patch does, not why.

Check the subject line.

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

* Re: [PATCH] Make is_signed_type() simpler
  2019-09-29 20:21 ` Rasmus Villemoes
@ 2019-09-29 21:09   ` Alexey Dobriyan
  2019-09-30  6:16     ` Rasmus Villemoes
  0 siblings, 1 reply; 8+ messages in thread
From: Alexey Dobriyan @ 2019-09-29 21:09 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: akpm, linux-kernel, jani.nikula, joonas.lahtinen, rodrigo.vivi,
	intel-gfx, rostedt, mingo

On Sun, Sep 29, 2019 at 10:21:48PM +0200, Rasmus Villemoes wrote:
> On 29/09/2019 22.06, Alexey Dobriyan wrote:
> > * Simply compare -1 with 0,
> > * Drop unnecessary parenthesis sets
> > 
> > -#define is_signed_type(type)       (((type)(-1)) < (type)1)
> > +#define is_signed_type(type)       ((type)-1 < 0)
> 
> NAK. I wrote it that way to avoid -Wtautological-compare when type is
> unsigned.

Was is W=1?

godbolt doesn't show it with just -Wall

	https://godbolt.org/z/kCA7mm

And the warning which found i915 case is -Wextra not -Wtautological-compare.

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

* Re: [PATCH] Make is_signed_type() simpler
  2019-09-29 20:44   ` Alexey Dobriyan
@ 2019-09-29 21:16     ` Steven Rostedt
  0 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2019-09-29 21:16 UTC (permalink / raw)
  To: Alexey Dobriyan
  Cc: akpm, linux-kernel, jani.nikula, joonas.lahtinen, rodrigo.vivi,
	intel-gfx, mingo, linux

On Sun, 29 Sep 2019 23:44:24 +0300
Alexey Dobriyan <adobriyan@gmail.com> wrote:

> On Sun, Sep 29, 2019 at 04:15:31PM -0400, Steven Rostedt wrote:
> > On Sun, 29 Sep 2019 23:06:19 +0300
> > Alexey Dobriyan <adobriyan@gmail.com> wrote:
> >   
> > > * Simply compare -1 with 0,
> > > * Drop unnecessary parenthesis sets
> > > 
> > > New macro leaves pointer as "unsigned type" but gives a warning,
> > > which should be fine because asking whether a pointer is signed is
> > > strange question.
> > > 
> > > I'm not sure what's going on in the i915 driver, it is shipping kernel
> > > pointers to userspace.  
> > 
> > This tells us what the patch does, not why.  
> 
> Check the subject line.

I don't see how it's simpler.

-#define is_signed_type(type)	(((type)(-1)) < (type)1)
+#define is_signed_type(type)	((type)-1 < 0)
 

Requires more rational that "make it simpler". Rewriting futex or tty
layer code would be something I would love to see, but just replacing
"(type)1" with "0" isn't worth the churn.

-- Steve

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

* Re: [PATCH] Make is_signed_type() simpler
  2019-09-29 21:09   ` Alexey Dobriyan
@ 2019-09-30  6:16     ` Rasmus Villemoes
  0 siblings, 0 replies; 8+ messages in thread
From: Rasmus Villemoes @ 2019-09-30  6:16 UTC (permalink / raw)
  To: Alexey Dobriyan
  Cc: akpm, linux-kernel, jani.nikula, joonas.lahtinen, rodrigo.vivi,
	intel-gfx, rostedt, mingo

On 29/09/2019 23.09, Alexey Dobriyan wrote:
> On Sun, Sep 29, 2019 at 10:21:48PM +0200, Rasmus Villemoes wrote:
>> On 29/09/2019 22.06, Alexey Dobriyan wrote:
>>> * Simply compare -1 with 0,
>>> * Drop unnecessary parenthesis sets
>>>
>>> -#define is_signed_type(type)       (((type)(-1)) < (type)1)
>>> +#define is_signed_type(type)       ((type)-1 < 0)
>>
>> NAK. I wrote it that way to avoid -Wtautological-compare when type is
>> unsigned.
> 
> Was is W=1?
> 
> godbolt doesn't show it with just -Wall
> 
> 	https://godbolt.org/z/kCA7mm
> 
> And the warning which found i915 case is -Wextra not -Wtautological-compare.
> 

Yeah, it's usually disabled/not enabled in the kernel. I did most of the
prototyping/testing in userspace with my default Cflags, and decided to
keep it this way in case somebody copy-pastes it to a code base that
does enable -Wtautological-compare. I see it's been copy-pasted to
tools/, who knows what they do. IMO, "it may be copy-pasted" would not
be valid reason for a transform the other way, but I really don't see a
reason for changing it now. Especially since it seems to require some
tree-wide adaptation.

Rasmus

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

* RE: [Intel-gfx] [PATCH] Make is_signed_type() simpler
  2019-09-29 20:06 [PATCH] Make is_signed_type() simpler Alexey Dobriyan
  2019-09-29 20:15 ` Steven Rostedt
  2019-09-29 20:21 ` Rasmus Villemoes
@ 2019-09-30 18:34 ` Ruhl, Michael J
  2 siblings, 0 replies; 8+ messages in thread
From: Ruhl, Michael J @ 2019-09-30 18:34 UTC (permalink / raw)
  To: Alexey Dobriyan, akpm; +Cc: intel-gfx, linux, linux-kernel, rostedt, mingo

>-----Original Message-----
>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
>Alexey Dobriyan
>Sent: Sunday, September 29, 2019 4:06 PM
>To: akpm@linux-foundation.org
>Cc: intel-gfx@lists.freedesktop.org; linux@rasmusvillemoes.dk; linux-
>kernel@vger.kernel.org; rostedt@goodmis.org; mingo@redhat.com
>Subject: [Intel-gfx] [PATCH] Make is_signed_type() simpler
>
>* Simply compare -1 with 0,
>* Drop unnecessary parenthesis sets
>
>New macro leaves pointer as "unsigned type" but gives a warning,
>which should be fine because asking whether a pointer is signed is
>strange question.
>
>I'm not sure what's going on in the i915 driver, it is shipping kernel
>pointers to userspace.
>
>Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
>---
>
> drivers/gpu/drm/i915/i915_trace.h |   86 +++++++++++++++++++--------------
>-----
> include/linux/overflow.h          |    2
> include/linux/trace_events.h      |    2
> tools/include/linux/overflow.h    |    2
> 4 files changed, 46 insertions(+), 46 deletions(-)
>
>--- a/drivers/gpu/drm/i915/i915_trace.h
>+++ b/drivers/gpu/drm/i915/i915_trace.h
>@@ -419,16 +419,16 @@ TRACE_EVENT(i915_gem_object_create,
> 	    TP_ARGS(obj),
>
> 	    TP_STRUCT__entry(
>-			     __field(struct drm_i915_gem_object *, obj)
>+			     __field(unsigned long, obj)
> 			     __field(u64, size)
> 			     ),
>
> 	    TP_fast_assign(
>-			   __entry->obj = obj;
>+			   __entry->obj = (unsigned long)obj;
> 			   __entry->size = obj->base.size;
> 			   ),
>
>-	    TP_printk("obj=%p, size=0x%llx", __entry->obj, __entry->size)
>+	    TP_printk("obj=%p, size=0x%llx", (void *)__entry->obj, __entry-

Looking up the printk format specifiers, I see:

%p

Pointers printed without a specifier extension (i.e unadorned %p) are
hashed to give a unique identifier without leaking kernel addresses to user
space.

This would seem to me that we this code is correct.  Am I misreading this?

Mike


>>size)
> );
>
> TRACE_EVENT(i915_gem_shrink,
>@@ -456,25 +456,25 @@ TRACE_EVENT(i915_vma_bind,
> 	    TP_ARGS(vma, flags),
>
> 	    TP_STRUCT__entry(
>-			     __field(struct drm_i915_gem_object *, obj)
>-			     __field(struct i915_address_space *, vm)
>+			     __field(unsigned long, obj)
>+			     __field(unsigned long, vm)
> 			     __field(u64, offset)
> 			     __field(u64, size)
> 			     __field(unsigned, flags)
> 			     ),
>
> 	    TP_fast_assign(
>-			   __entry->obj = vma->obj;
>-			   __entry->vm = vma->vm;
>+			   __entry->obj = (unsigned long)vma->obj;
>+			   __entry->vm = (unsigned long)vma->vm;
> 			   __entry->offset = vma->node.start;
> 			   __entry->size = vma->node.size;
> 			   __entry->flags = flags;
> 			   ),
>
> 	    TP_printk("obj=%p, offset=0x%016llx size=0x%llx%s vm=%p",
>-		      __entry->obj, __entry->offset, __entry->size,
>+		      (void *)__entry->obj, __entry->offset, __entry->size,
> 		      __entry->flags & PIN_MAPPABLE ? ", mappable" : "",
>-		      __entry->vm)
>+		      (void *)__entry->vm)
> );
>
> TRACE_EVENT(i915_vma_unbind,
>@@ -482,21 +482,21 @@ TRACE_EVENT(i915_vma_unbind,
> 	    TP_ARGS(vma),
>
> 	    TP_STRUCT__entry(
>-			     __field(struct drm_i915_gem_object *, obj)
>-			     __field(struct i915_address_space *, vm)
>+			     __field(unsigned long, obj)
>+			     __field(unsigned long, vm)
> 			     __field(u64, offset)
> 			     __field(u64, size)
> 			     ),
>
> 	    TP_fast_assign(
>-			   __entry->obj = vma->obj;
>-			   __entry->vm = vma->vm;
>+			   __entry->obj = (unsigned long)vma->obj;
>+			   __entry->vm = (unsigned long)vma->vm;
> 			   __entry->offset = vma->node.start;
> 			   __entry->size = vma->node.size;
> 			   ),
>
> 	    TP_printk("obj=%p, offset=0x%016llx size=0x%llx vm=%p",
>-		      __entry->obj, __entry->offset, __entry->size, __entry-
>>vm)
>+		      (void *)__entry->obj, __entry->offset, __entry->size,
>(void *)__entry->vm)
> );
>
> TRACE_EVENT(i915_gem_object_pwrite,
>@@ -504,19 +504,19 @@ TRACE_EVENT(i915_gem_object_pwrite,
> 	    TP_ARGS(obj, offset, len),
>
> 	    TP_STRUCT__entry(
>-			     __field(struct drm_i915_gem_object *, obj)
>+			     __field(unsigned long, obj)
> 			     __field(u64, offset)
> 			     __field(u64, len)
> 			     ),
>
> 	    TP_fast_assign(
>-			   __entry->obj = obj;
>+			   __entry->obj = (unsigned long)obj;
> 			   __entry->offset = offset;
> 			   __entry->len = len;
> 			   ),
>
> 	    TP_printk("obj=%p, offset=0x%llx, len=0x%llx",
>-		      __entry->obj, __entry->offset, __entry->len)
>+		      (void *)__entry->obj, __entry->offset, __entry->len)
> );
>
> TRACE_EVENT(i915_gem_object_pread,
>@@ -524,19 +524,19 @@ TRACE_EVENT(i915_gem_object_pread,
> 	    TP_ARGS(obj, offset, len),
>
> 	    TP_STRUCT__entry(
>-			     __field(struct drm_i915_gem_object *, obj)
>+			     __field(unsigned long, obj)
> 			     __field(u64, offset)
> 			     __field(u64, len)
> 			     ),
>
> 	    TP_fast_assign(
>-			   __entry->obj = obj;
>+			   __entry->obj = (unsigned long)obj;
> 			   __entry->offset = offset;
> 			   __entry->len = len;
> 			   ),
>
> 	    TP_printk("obj=%p, offset=0x%llx, len=0x%llx",
>-		      __entry->obj, __entry->offset, __entry->len)
>+		      (void *)__entry->obj, __entry->offset, __entry->len)
> );
>
> TRACE_EVENT(i915_gem_object_fault,
>@@ -544,21 +544,21 @@ TRACE_EVENT(i915_gem_object_fault,
> 	    TP_ARGS(obj, index, gtt, write),
>
> 	    TP_STRUCT__entry(
>-			     __field(struct drm_i915_gem_object *, obj)
>+			     __field(unsigned long, obj)
> 			     __field(u64, index)
> 			     __field(bool, gtt)
> 			     __field(bool, write)
> 			     ),
>
> 	    TP_fast_assign(
>-			   __entry->obj = obj;
>+			   __entry->obj = (unsigned long)obj;
> 			   __entry->index = index;
> 			   __entry->gtt = gtt;
> 			   __entry->write = write;
> 			   ),
>
> 	    TP_printk("obj=%p, %s index=%llu %s",
>-		      __entry->obj,
>+		      (void *)__entry->obj,
> 		      __entry->gtt ? "GTT" : "CPU",
> 		      __entry->index,
> 		      __entry->write ? ", writable" : "")
>@@ -569,14 +569,14 @@ DECLARE_EVENT_CLASS(i915_gem_object,
> 	    TP_ARGS(obj),
>
> 	    TP_STRUCT__entry(
>-			     __field(struct drm_i915_gem_object *, obj)
>+			     __field(unsigned long, obj)
> 			     ),
>
> 	    TP_fast_assign(
>-			   __entry->obj = obj;
>+			   __entry->obj = (unsigned long)obj;
> 			   ),
>
>-	    TP_printk("obj=%p", __entry->obj)
>+	    TP_printk("obj=%p", (void *)__entry->obj)
> );
>
> DEFINE_EVENT(i915_gem_object, i915_gem_object_clflush,
>@@ -595,7 +595,7 @@ TRACE_EVENT(i915_gem_evict,
>
> 	    TP_STRUCT__entry(
> 			     __field(u32, dev)
>-			     __field(struct i915_address_space *, vm)
>+			     __field(unsigned long, vm)
> 			     __field(u64, size)
> 			     __field(u64, align)
> 			     __field(unsigned int, flags)
>@@ -603,14 +603,14 @@ TRACE_EVENT(i915_gem_evict,
>
> 	    TP_fast_assign(
> 			   __entry->dev = vm->i915->drm.primary->index;
>-			   __entry->vm = vm;
>+			   __entry->vm = (unsigned long)vm;
> 			   __entry->size = size;
> 			   __entry->align = align;
> 			   __entry->flags = flags;
> 			  ),
>
> 	    TP_printk("dev=%d, vm=%p, size=0x%llx, align=0x%llx %s",
>-		      __entry->dev, __entry->vm, __entry->size, __entry-
>>align,
>+		      __entry->dev, (void *)__entry->vm, __entry->size,
>__entry->align,
> 		      __entry->flags & PIN_MAPPABLE ? ", mappable" : "")
> );
>
>@@ -620,7 +620,7 @@ TRACE_EVENT(i915_gem_evict_node,
>
> 	    TP_STRUCT__entry(
> 			     __field(u32, dev)
>-			     __field(struct i915_address_space *, vm)
>+			     __field(unsigned long, vm)
> 			     __field(u64, start)
> 			     __field(u64, size)
> 			     __field(unsigned long, color)
>@@ -629,7 +629,7 @@ TRACE_EVENT(i915_gem_evict_node,
>
> 	    TP_fast_assign(
> 			   __entry->dev = vm->i915->drm.primary->index;
>-			   __entry->vm = vm;
>+			   __entry->vm = (unsigned long)vm;
> 			   __entry->start = node->start;
> 			   __entry->size = node->size;
> 			   __entry->color = node->color;
>@@ -637,7 +637,7 @@ TRACE_EVENT(i915_gem_evict_node,
> 			  ),
>
> 	    TP_printk("dev=%d, vm=%p, start=0x%llx size=0x%llx, color=0x%lx,
>flags=%x",
>-		      __entry->dev, __entry->vm,
>+		      __entry->dev, (void *)__entry->vm,
> 		      __entry->start, __entry->size,
> 		      __entry->color, __entry->flags)
> );
>@@ -648,15 +648,15 @@ TRACE_EVENT(i915_gem_evict_vm,
>
> 	    TP_STRUCT__entry(
> 			     __field(u32, dev)
>-			     __field(struct i915_address_space *, vm)
>+			     __field(unsigned long, vm)
> 			    ),
>
> 	    TP_fast_assign(
> 			   __entry->dev = vm->i915->drm.primary->index;
>-			   __entry->vm = vm;
>+			   __entry->vm = (unsigned long)vm;
> 			  ),
>
>-	    TP_printk("dev=%d, vm=%p", __entry->dev, __entry->vm)
>+	    TP_printk("dev=%d, vm=%p", __entry->dev, (void *)__entry->vm)
> );
>
> TRACE_EVENT(i915_request_queue,
>@@ -922,16 +922,16 @@ DECLARE_EVENT_CLASS(i915_ppgtt,
> 	TP_ARGS(vm),
>
> 	TP_STRUCT__entry(
>-			__field(struct i915_address_space *, vm)
>+			__field(unsigned long, vm)
> 			__field(u32, dev)
> 	),
>
> 	TP_fast_assign(
>-			__entry->vm = vm;
>+			__entry->vm = (unsigned long)vm;
> 			__entry->dev = vm->i915->drm.primary->index;
> 	),
>
>-	TP_printk("dev=%u, vm=%p", __entry->dev, __entry->vm)
>+	TP_printk("dev=%u, vm=%p", __entry->dev, (void *)__entry->vm)
> )
>
> DEFINE_EVENT(i915_ppgtt, i915_ppgtt_create,
>@@ -957,20 +957,20 @@ DECLARE_EVENT_CLASS(i915_context,
>
> 	TP_STRUCT__entry(
> 			__field(u32, dev)
>-			__field(struct i915_gem_context *, ctx)
>+			__field(unsigned long, ctx)
> 			__field(u32, hw_id)
>-			__field(struct i915_address_space *, vm)
>+			__field(unsigned long, vm)
> 	),
>
> 	TP_fast_assign(
> 			__entry->dev = ctx->i915->drm.primary->index;
>-			__entry->ctx = ctx;
>+			__entry->ctx = (unsigned long)ctx;
> 			__entry->hw_id = ctx->hw_id;
>-			__entry->vm = ctx->vm;
>+			__entry->vm = (unsigned long)ctx->vm;
> 	),
>
> 	TP_printk("dev=%u, ctx=%p, ctx_vm=%p, hw_id=%u",
>-		  __entry->dev, __entry->ctx, __entry->vm, __entry->hw_id)
>+		  __entry->dev, (void *)__entry->ctx, (void *)__entry->vm,
>__entry->hw_id)
> )
>
> DEFINE_EVENT(i915_context, i915_context_create,
>--- a/include/linux/overflow.h
>+++ b/include/linux/overflow.h
>@@ -31,7 +31,7 @@
>  * https://mail-index.netbsd.org/tech-misc/2007/02/05/0000.html -
>  * credit to Christian Biere.
>  */
>-#define is_signed_type(type)       (((type)(-1)) < (type)1)
>+#define is_signed_type(type)       ((type)-1 < 0)
> #define __type_half_max(type) ((type)1 << (8*sizeof(type) - 1 -
>is_signed_type(type)))
> #define type_max(T) ((T)((__type_half_max(T) - 1) + __type_half_max(T)))
> #define type_min(T) ((T)((T)-type_max(T)-(T)1))
>--- a/include/linux/trace_events.h
>+++ b/include/linux/trace_events.h
>@@ -546,7 +546,7 @@ extern int trace_add_event_call(struct
>trace_event_call *call);
> extern int trace_remove_event_call(struct trace_event_call *call);
> extern int trace_event_get_offsets(struct trace_event_call *call);
>
>-#define is_signed_type(type)	(((type)(-1)) < (type)1)
>+#define is_signed_type(type)	((type)-1 < 0)
>
> int ftrace_set_clr_event(struct trace_array *tr, char *buf, int set);
> int trace_set_clr_event(const char *system, const char *event, int set);
>--- a/tools/include/linux/overflow.h
>+++ b/tools/include/linux/overflow.h
>@@ -31,7 +31,7 @@
>  * https://mail-index.netbsd.org/tech-misc/2007/02/05/0000.html -
>  * credit to Christian Biere.
>  */
>-#define is_signed_type(type)       (((type)(-1)) < (type)1)
>+#define is_signed_type(type)       ((type)-1 < 0)
> #define __type_half_max(type) ((type)1 << (8*sizeof(type) - 1 -
>is_signed_type(type)))
> #define type_max(T) ((T)((__type_half_max(T) - 1) + __type_half_max(T)))
> #define type_min(T) ((T)((T)-type_max(T)-(T)1))
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2019-09-30 20:40 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-29 20:06 [PATCH] Make is_signed_type() simpler Alexey Dobriyan
2019-09-29 20:15 ` Steven Rostedt
2019-09-29 20:44   ` Alexey Dobriyan
2019-09-29 21:16     ` Steven Rostedt
2019-09-29 20:21 ` Rasmus Villemoes
2019-09-29 21:09   ` Alexey Dobriyan
2019-09-30  6:16     ` Rasmus Villemoes
2019-09-30 18:34 ` [Intel-gfx] " Ruhl, Michael J

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).