Re: [PATCH v4 04/79] drm_mode.h: use __u32 and __u64 from linux/types.h

[Emil Velikov]

On 21 October 2015 at 17:27, Alex Deucher  wrote:
> On Wed, Oct 21, 2015 at 12:21 PM, Emil Velikov  
> wrote:
>> On 21 October 2015 at 16:18, Alex Deucher  wrote:
>>> On Wed, Oct 21, 2015 at 11:09 AM, Emil Velikov  
>>> wrote:
 Hi Alex,

 On 15 October 2015 at 14:48, Mikko Rapeli  wrote:
> On Thu, Oct 15, 2015 at 09:32:10AM -0400, Alex Deucher wrote:
>> On Thu, Oct 15, 2015 at 1:55 AM, Mikko Rapeli  
>> wrote:
>> > Fixes userspace compilation error:
>> >
>> > drm/drm_mode.h:472:2: error: unknown type name ‘uint32_t’
>> >
>> > Signed-off-by: Mikko Rapeli 
>>
>> NACK on all these type conversions.  This has not been a problem for
>> years and years and the result looks terrible.
>
> Documentation/CodingStyle, section 5
>
>  (e) Types safe for use in userspace.
>
>  In certain structures which are visible to userspace, we cannot
>  require C99 types and cannot use the 'u32' form above. Thus, we
>  use __u32 and similar types in all structures which are shared
>  with userspace.
>
> I have only been looking at kernel headers from userspace occationally in
> the past 10 years and had a several cases where the provided headers did
> not compile when included into trivial programs trying to use the structs
> for an ioctl() for example. This long lasting problem triggered me to 
> write
> a test for this and provide these fixes too. In previous reviews usage
> of  and its types in kernel headers was already NACK'ed
> so I changed several places from uint32_t's to __u32.
>
> With these changes it is btw trivial now to add a grep test the there
> are no uint32_t's in include/uapi/ anymore, thus enforcing that coding 
> style
> rule.
>
 Based of the reply from Mikko, can you please elaborate your concern ?
 Are you thinking about some corner case where this may cause breakage,
 or it's solely on stylistic point of view ?
>>>
>>> Style.
>>>
>> In that case shouldn't the kernel coding style (not to mention the
>> issues with stdint.h types Linus has brought a while back) be
>> considered/prevail ?
>
> I don't really have a strong opinion one way or another at this point.
> The types just look ugly.
>
Fwiw I don't find them too appealing either. But the alternative did
not fare so well :(

>>

 Over the last few years we've been doing some ad-hoc 'synchronisation'
 with the headers in libdrm, and this will get us one step closer to
 doing things properly.
>>>
>>> How does this affect libdrm one way or another?
>>>
>> In a strange way. Some (most?) distributions do not ship the uapi
>> headers from the kernel, but rely on the ones in libdrm. Due to
>> various issues (as addressed in the series) we have not synchronised
>> the headers, thus users such as the gallium radeon winsys has ended up
>> redefining/duplicating internally.
>
> The problem is that you generally need the latest headers to support
> the latest features in the user mode stacks, so if the user tries to
> build against an older kernel, they will be missing definitions.
>
Pretty much that's why we have the loose release criteria for libdrm.
Get the functionality upstream (mainline or -next), pull the changes
(sync) and release. As the sync has been busted for a while, people
are forced to do unpleasant things.

-Emil
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 04/79] drm_mode.h: use __u32 and __u64 from linux/types.h

[Alex Deucher]

On Wed, Oct 21, 2015 at 12:21 PM, Emil Velikov  wrote:
> On 21 October 2015 at 16:18, Alex Deucher  wrote:
>> On Wed, Oct 21, 2015 at 11:09 AM, Emil Velikov  
>> wrote:
>>> Hi Alex,
>>>
>>> On 15 October 2015 at 14:48, Mikko Rapeli  wrote:
 On Thu, Oct 15, 2015 at 09:32:10AM -0400, Alex Deucher wrote:
> On Thu, Oct 15, 2015 at 1:55 AM, Mikko Rapeli  wrote:
> > Fixes userspace compilation error:
> >
> > drm/drm_mode.h:472:2: error: unknown type name ‘uint32_t’
> >
> > Signed-off-by: Mikko Rapeli 
>
> NACK on all these type conversions.  This has not been a problem for
> years and years and the result looks terrible.

 Documentation/CodingStyle, section 5

  (e) Types safe for use in userspace.

  In certain structures which are visible to userspace, we cannot
  require C99 types and cannot use the 'u32' form above. Thus, we
  use __u32 and similar types in all structures which are shared
  with userspace.

 I have only been looking at kernel headers from userspace occationally in
 the past 10 years and had a several cases where the provided headers did
 not compile when included into trivial programs trying to use the structs
 for an ioctl() for example. This long lasting problem triggered me to write
 a test for this and provide these fixes too. In previous reviews usage
 of  and its types in kernel headers was already NACK'ed
 so I changed several places from uint32_t's to __u32.

 With these changes it is btw trivial now to add a grep test the there
 are no uint32_t's in include/uapi/ anymore, thus enforcing that coding 
 style
 rule.

>>> Based of the reply from Mikko, can you please elaborate your concern ?
>>> Are you thinking about some corner case where this may cause breakage,
>>> or it's solely on stylistic point of view ?
>>
>> Style.
>>
> In that case shouldn't the kernel coding style (not to mention the
> issues with stdint.h types Linus has brought a while back) be
> considered/prevail ?

I don't really have a strong opinion one way or another at this point.
The types just look ugly.

>
>>>
>>> Over the last few years we've been doing some ad-hoc 'synchronisation'
>>> with the headers in libdrm, and this will get us one step closer to
>>> doing things properly.
>>
>> How does this affect libdrm one way or another?
>>
> In a strange way. Some (most?) distributions do not ship the uapi
> headers from the kernel, but rely on the ones in libdrm. Due to
> various issues (as addressed in the series) we have not synchronised
> the headers, thus users such as the gallium radeon winsys has ended up
> redefining/duplicating internally.

The problem is that you generally need the latest headers to support
the latest features in the user mode stacks, so if the user tries to
build against an older kernel, they will be missing definitions.

Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 04/79] drm_mode.h: use __u32 and __u64 from linux/types.h

[Emil Velikov]

On 21 October 2015 at 16:18, Alex Deucher  wrote:
> On Wed, Oct 21, 2015 at 11:09 AM, Emil Velikov  
> wrote:
>> Hi Alex,
>>
>> On 15 October 2015 at 14:48, Mikko Rapeli  wrote:
>>> On Thu, Oct 15, 2015 at 09:32:10AM -0400, Alex Deucher wrote:
 On Thu, Oct 15, 2015 at 1:55 AM, Mikko Rapeli  wrote:
 > Fixes userspace compilation error:
 >
 > drm/drm_mode.h:472:2: error: unknown type name ‘uint32_t’
 >
 > Signed-off-by: Mikko Rapeli 

 NACK on all these type conversions.  This has not been a problem for
 years and years and the result looks terrible.
>>>
>>> Documentation/CodingStyle, section 5
>>>
>>>  (e) Types safe for use in userspace.
>>>
>>>  In certain structures which are visible to userspace, we cannot
>>>  require C99 types and cannot use the 'u32' form above. Thus, we
>>>  use __u32 and similar types in all structures which are shared
>>>  with userspace.
>>>
>>> I have only been looking at kernel headers from userspace occationally in
>>> the past 10 years and had a several cases where the provided headers did
>>> not compile when included into trivial programs trying to use the structs
>>> for an ioctl() for example. This long lasting problem triggered me to write
>>> a test for this and provide these fixes too. In previous reviews usage
>>> of  and its types in kernel headers was already NACK'ed
>>> so I changed several places from uint32_t's to __u32.
>>>
>>> With these changes it is btw trivial now to add a grep test the there
>>> are no uint32_t's in include/uapi/ anymore, thus enforcing that coding style
>>> rule.
>>>
>> Based of the reply from Mikko, can you please elaborate your concern ?
>> Are you thinking about some corner case where this may cause breakage,
>> or it's solely on stylistic point of view ?
>
> Style.
>
In that case shouldn't the kernel coding style (not to mention the
issues with stdint.h types Linus has brought a while back) be
considered/prevail ?

>>
>> Over the last few years we've been doing some ad-hoc 'synchronisation'
>> with the headers in libdrm, and this will get us one step closer to
>> doing things properly.
>
> How does this affect libdrm one way or another?
>
In a strange way. Some (most?) distributions do not ship the uapi
headers from the kernel, but rely on the ones in libdrm. Due to
various issues (as addressed in the series) we have not synchronised
the headers, thus users such as the gallium radeon winsys has ended up
redefining/duplicating internally.

Thanks
Emil
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 04/79] drm_mode.h: use __u32 and __u64 from linux/types.h

[Alex Deucher]

On Wed, Oct 21, 2015 at 11:09 AM, Emil Velikov  wrote:
> Hi Alex,
>
> On 15 October 2015 at 14:48, Mikko Rapeli  wrote:
>> On Thu, Oct 15, 2015 at 09:32:10AM -0400, Alex Deucher wrote:
>>> On Thu, Oct 15, 2015 at 1:55 AM, Mikko Rapeli  wrote:
>>> > Fixes userspace compilation error:
>>> >
>>> > drm/drm_mode.h:472:2: error: unknown type name ‘uint32_t’
>>> >
>>> > Signed-off-by: Mikko Rapeli 
>>>
>>> NACK on all these type conversions.  This has not been a problem for
>>> years and years and the result looks terrible.
>>
>> Documentation/CodingStyle, section 5
>>
>>  (e) Types safe for use in userspace.
>>
>>  In certain structures which are visible to userspace, we cannot
>>  require C99 types and cannot use the 'u32' form above. Thus, we
>>  use __u32 and similar types in all structures which are shared
>>  with userspace.
>>
>> I have only been looking at kernel headers from userspace occationally in
>> the past 10 years and had a several cases where the provided headers did
>> not compile when included into trivial programs trying to use the structs
>> for an ioctl() for example. This long lasting problem triggered me to write
>> a test for this and provide these fixes too. In previous reviews usage
>> of  and its types in kernel headers was already NACK'ed
>> so I changed several places from uint32_t's to __u32.
>>
>> With these changes it is btw trivial now to add a grep test the there
>> are no uint32_t's in include/uapi/ anymore, thus enforcing that coding style
>> rule.
>>
> Based of the reply from Mikko, can you please elaborate your concern ?
> Are you thinking about some corner case where this may cause breakage,
> or it's solely on stylistic point of view ?

Style.

>
> Over the last few years we've been doing some ad-hoc 'synchronisation'
> with the headers in libdrm, and this will get us one step closer to
> doing things properly.

How does this affect libdrm one way or another?

Alex

>
> Fwiw I fully support these changes, as does Gustavo for exynos and Rob for 
> msm.
> Reviewed-by: Emil Velikov 
>
> Thanks
> Emil
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 04/79] drm_mode.h: use __u32 and __u64 from linux/types.h

[Emil Velikov]

Hi Alex,

On 15 October 2015 at 14:48, Mikko Rapeli  wrote:
> On Thu, Oct 15, 2015 at 09:32:10AM -0400, Alex Deucher wrote:
>> On Thu, Oct 15, 2015 at 1:55 AM, Mikko Rapeli  wrote:
>> > Fixes userspace compilation error:
>> >
>> > drm/drm_mode.h:472:2: error: unknown type name ‘uint32_t’
>> >
>> > Signed-off-by: Mikko Rapeli 
>>
>> NACK on all these type conversions.  This has not been a problem for
>> years and years and the result looks terrible.
>
> Documentation/CodingStyle, section 5
>
>  (e) Types safe for use in userspace.
>
>  In certain structures which are visible to userspace, we cannot
>  require C99 types and cannot use the 'u32' form above. Thus, we
>  use __u32 and similar types in all structures which are shared
>  with userspace.
>
> I have only been looking at kernel headers from userspace occationally in
> the past 10 years and had a several cases where the provided headers did
> not compile when included into trivial programs trying to use the structs
> for an ioctl() for example. This long lasting problem triggered me to write
> a test for this and provide these fixes too. In previous reviews usage
> of  and its types in kernel headers was already NACK'ed
> so I changed several places from uint32_t's to __u32.
>
> With these changes it is btw trivial now to add a grep test the there
> are no uint32_t's in include/uapi/ anymore, thus enforcing that coding style
> rule.
>
Based of the reply from Mikko, can you please elaborate your concern ?
Are you thinking about some corner case where this may cause breakage,
or it's solely on stylistic point of view ?

Over the last few years we've been doing some ad-hoc 'synchronisation'
with the headers in libdrm, and this will get us one step closer to
doing things properly.

Fwiw I fully support these changes, as does Gustavo for exynos and Rob for msm.
Reviewed-by: Emil Velikov 

Thanks
Emil
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 04/79] drm_mode.h: use __u32 and __u64 from linux/types.h

[Alex Deucher]

On Wed, Oct 21, 2015 at 11:09 AM, Emil Velikov  wrote:
> Hi Alex,
>
> On 15 October 2015 at 14:48, Mikko Rapeli  wrote:
>> On Thu, Oct 15, 2015 at 09:32:10AM -0400, Alex Deucher wrote:
>>> On Thu, Oct 15, 2015 at 1:55 AM, Mikko Rapeli  wrote:
>>> > Fixes userspace compilation error:
>>> >
>>> > drm/drm_mode.h:472:2: error: unknown type name ‘uint32_t’
>>> >
>>> > Signed-off-by: Mikko Rapeli 
>>>
>>> NACK on all these type conversions.  This has not been a problem for
>>> years and years and the result looks terrible.
>>
>> Documentation/CodingStyle, section 5
>>
>>  (e) Types safe for use in userspace.
>>
>>  In certain structures which are visible to userspace, we cannot
>>  require C99 types and cannot use the 'u32' form above. Thus, we
>>  use __u32 and similar types in all structures which are shared
>>  with userspace.
>>
>> I have only been looking at kernel headers from userspace occationally in
>> the past 10 years and had a several cases where the provided headers did
>> not compile when included into trivial programs trying to use the structs
>> for an ioctl() for example. This long lasting problem triggered me to write
>> a test for this and provide these fixes too. In previous reviews usage
>> of  and its types in kernel headers was already NACK'ed
>> so I changed several places from uint32_t's to __u32.
>>
>> With these changes it is btw trivial now to add a grep test the there
>> are no uint32_t's in include/uapi/ anymore, thus enforcing that coding style
>> rule.
>>
> Based of the reply from Mikko, can you please elaborate your concern ?
> Are you thinking about some corner case where this may cause breakage,
> or it's solely on stylistic point of view ?

Style.

>
> Over the last few years we've been doing some ad-hoc 'synchronisation'
> with the headers in libdrm, and this will get us one step closer to
> doing things properly.

How does this affect libdrm one way or another?

Alex

>
> Fwiw I fully support these changes, as does Gustavo for exynos and Rob for 
> msm.
> Reviewed-by: Emil Velikov 
>
> Thanks
> Emil
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 04/79] drm_mode.h: use __u32 and __u64 from linux/types.h

[Alex Deucher]

On Wed, Oct 21, 2015 at 12:21 PM, Emil Velikov  wrote:
> On 21 October 2015 at 16:18, Alex Deucher  wrote:
>> On Wed, Oct 21, 2015 at 11:09 AM, Emil Velikov  
>> wrote:
>>> Hi Alex,
>>>
>>> On 15 October 2015 at 14:48, Mikko Rapeli  wrote:
 On Thu, Oct 15, 2015 at 09:32:10AM -0400, Alex Deucher wrote:
> On Thu, Oct 15, 2015 at 1:55 AM, Mikko Rapeli  wrote:
> > Fixes userspace compilation error:
> >
> > drm/drm_mode.h:472:2: error: unknown type name ‘uint32_t’
> >
> > Signed-off-by: Mikko Rapeli 
>
> NACK on all these type conversions.  This has not been a problem for
> years and years and the result looks terrible.

 Documentation/CodingStyle, section 5

  (e) Types safe for use in userspace.

  In certain structures which are visible to userspace, we cannot
  require C99 types and cannot use the 'u32' form above. Thus, we
  use __u32 and similar types in all structures which are shared
  with userspace.

 I have only been looking at kernel headers from userspace occationally in
 the past 10 years and had a several cases where the provided headers did
 not compile when included into trivial programs trying to use the structs
 for an ioctl() for example. This long lasting problem triggered me to write
 a test for this and provide these fixes too. In previous reviews usage
 of  and its types in kernel headers was already NACK'ed
 so I changed several places from uint32_t's to __u32.

 With these changes it is btw trivial now to add a grep test the there
 are no uint32_t's in include/uapi/ anymore, thus enforcing that coding 
 style
 rule.

>>> Based of the reply from Mikko, can you please elaborate your concern ?
>>> Are you thinking about some corner case where this may cause breakage,
>>> or it's solely on stylistic point of view ?
>>
>> Style.
>>
> In that case shouldn't the kernel coding style (not to mention the
> issues with stdint.h types Linus has brought a while back) be
> considered/prevail ?

I don't really have a strong opinion one way or another at this point.
The types just look ugly.

>
>>>
>>> Over the last few years we've been doing some ad-hoc 'synchronisation'
>>> with the headers in libdrm, and this will get us one step closer to
>>> doing things properly.
>>
>> How does this affect libdrm one way or another?
>>
> In a strange way. Some (most?) distributions do not ship the uapi
> headers from the kernel, but rely on the ones in libdrm. Due to
> various issues (as addressed in the series) we have not synchronised
> the headers, thus users such as the gallium radeon winsys has ended up
> redefining/duplicating internally.

The problem is that you generally need the latest headers to support
the latest features in the user mode stacks, so if the user tries to
build against an older kernel, they will be missing definitions.

Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 04/79] drm_mode.h: use __u32 and __u64 from linux/types.h

[Emil Velikov]

On 21 October 2015 at 16:18, Alex Deucher  wrote:
> On Wed, Oct 21, 2015 at 11:09 AM, Emil Velikov  
> wrote:
>> Hi Alex,
>>
>> On 15 October 2015 at 14:48, Mikko Rapeli  wrote:
>>> On Thu, Oct 15, 2015 at 09:32:10AM -0400, Alex Deucher wrote:
 On Thu, Oct 15, 2015 at 1:55 AM, Mikko Rapeli  wrote:
 > Fixes userspace compilation error:
 >
 > drm/drm_mode.h:472:2: error: unknown type name ‘uint32_t’
 >
 > Signed-off-by: Mikko Rapeli 

 NACK on all these type conversions.  This has not been a problem for
 years and years and the result looks terrible.
>>>
>>> Documentation/CodingStyle, section 5
>>>
>>>  (e) Types safe for use in userspace.
>>>
>>>  In certain structures which are visible to userspace, we cannot
>>>  require C99 types and cannot use the 'u32' form above. Thus, we
>>>  use __u32 and similar types in all structures which are shared
>>>  with userspace.
>>>
>>> I have only been looking at kernel headers from userspace occationally in
>>> the past 10 years and had a several cases where the provided headers did
>>> not compile when included into trivial programs trying to use the structs
>>> for an ioctl() for example. This long lasting problem triggered me to write
>>> a test for this and provide these fixes too. In previous reviews usage
>>> of  and its types in kernel headers was already NACK'ed
>>> so I changed several places from uint32_t's to __u32.
>>>
>>> With these changes it is btw trivial now to add a grep test the there
>>> are no uint32_t's in include/uapi/ anymore, thus enforcing that coding style
>>> rule.
>>>
>> Based of the reply from Mikko, can you please elaborate your concern ?
>> Are you thinking about some corner case where this may cause breakage,
>> or it's solely on stylistic point of view ?
>
> Style.
>
In that case shouldn't the kernel coding style (not to mention the
issues with stdint.h types Linus has brought a while back) be
considered/prevail ?

>>
>> Over the last few years we've been doing some ad-hoc 'synchronisation'
>> with the headers in libdrm, and this will get us one step closer to
>> doing things properly.
>
> How does this affect libdrm one way or another?
>
In a strange way. Some (most?) distributions do not ship the uapi
headers from the kernel, but rely on the ones in libdrm. Due to
various issues (as addressed in the series) we have not synchronised
the headers, thus users such as the gallium radeon winsys has ended up
redefining/duplicating internally.

Thanks
Emil
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 04/79] drm_mode.h: use __u32 and __u64 from linux/types.h

[Emil Velikov]

Hi Alex,

On 15 October 2015 at 14:48, Mikko Rapeli  wrote:
> On Thu, Oct 15, 2015 at 09:32:10AM -0400, Alex Deucher wrote:
>> On Thu, Oct 15, 2015 at 1:55 AM, Mikko Rapeli  wrote:
>> > Fixes userspace compilation error:
>> >
>> > drm/drm_mode.h:472:2: error: unknown type name ‘uint32_t’
>> >
>> > Signed-off-by: Mikko Rapeli 
>>
>> NACK on all these type conversions.  This has not been a problem for
>> years and years and the result looks terrible.
>
> Documentation/CodingStyle, section 5
>
>  (e) Types safe for use in userspace.
>
>  In certain structures which are visible to userspace, we cannot
>  require C99 types and cannot use the 'u32' form above. Thus, we
>  use __u32 and similar types in all structures which are shared
>  with userspace.
>
> I have only been looking at kernel headers from userspace occationally in
> the past 10 years and had a several cases where the provided headers did
> not compile when included into trivial programs trying to use the structs
> for an ioctl() for example. This long lasting problem triggered me to write
> a test for this and provide these fixes too. In previous reviews usage
> of  and its types in kernel headers was already NACK'ed
> so I changed several places from uint32_t's to __u32.
>
> With these changes it is btw trivial now to add a grep test the there
> are no uint32_t's in include/uapi/ anymore, thus enforcing that coding style
> rule.
>
Based of the reply from Mikko, can you please elaborate your concern ?
Are you thinking about some corner case where this may cause breakage,
or it's solely on stylistic point of view ?

Over the last few years we've been doing some ad-hoc 'synchronisation'
with the headers in libdrm, and this will get us one step closer to
doing things properly.

Fwiw I fully support these changes, as does Gustavo for exynos and Rob for msm.
Reviewed-by: Emil Velikov 

Thanks
Emil
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 04/79] drm_mode.h: use __u32 and __u64 from linux/types.h

[Emil Velikov]

On 21 October 2015 at 17:27, Alex Deucher  wrote:
> On Wed, Oct 21, 2015 at 12:21 PM, Emil Velikov  
> wrote:
>> On 21 October 2015 at 16:18, Alex Deucher  wrote:
>>> On Wed, Oct 21, 2015 at 11:09 AM, Emil Velikov  
>>> wrote:
 Hi Alex,

 On 15 October 2015 at 14:48, Mikko Rapeli  wrote:
> On Thu, Oct 15, 2015 at 09:32:10AM -0400, Alex Deucher wrote:
>> On Thu, Oct 15, 2015 at 1:55 AM, Mikko Rapeli  
>> wrote:
>> > Fixes userspace compilation error:
>> >
>> > drm/drm_mode.h:472:2: error: unknown type name ‘uint32_t’
>> >
>> > Signed-off-by: Mikko Rapeli 
>>
>> NACK on all these type conversions.  This has not been a problem for
>> years and years and the result looks terrible.
>
> Documentation/CodingStyle, section 5
>
>  (e) Types safe for use in userspace.
>
>  In certain structures which are visible to userspace, we cannot
>  require C99 types and cannot use the 'u32' form above. Thus, we
>  use __u32 and similar types in all structures which are shared
>  with userspace.
>
> I have only been looking at kernel headers from userspace occationally in
> the past 10 years and had a several cases where the provided headers did
> not compile when included into trivial programs trying to use the structs
> for an ioctl() for example. This long lasting problem triggered me to 
> write
> a test for this and provide these fixes too. In previous reviews usage
> of  and its types in kernel headers was already NACK'ed
> so I changed several places from uint32_t's to __u32.
>
> With these changes it is btw trivial now to add a grep test the there
> are no uint32_t's in include/uapi/ anymore, thus enforcing that coding 
> style
> rule.
>
 Based of the reply from Mikko, can you please elaborate your concern ?
 Are you thinking about some corner case where this may cause breakage,
 or it's solely on stylistic point of view ?
>>>
>>> Style.
>>>
>> In that case shouldn't the kernel coding style (not to mention the
>> issues with stdint.h types Linus has brought a while back) be
>> considered/prevail ?
>
> I don't really have a strong opinion one way or another at this point.
> The types just look ugly.
>
Fwiw I don't find them too appealing either. But the alternative did
not fare so well :(

>>

 Over the last few years we've been doing some ad-hoc 'synchronisation'
 with the headers in libdrm, and this will get us one step closer to
 doing things properly.
>>>
>>> How does this affect libdrm one way or another?
>>>
>> In a strange way. Some (most?) distributions do not ship the uapi
>> headers from the kernel, but rely on the ones in libdrm. Due to
>> various issues (as addressed in the series) we have not synchronised
>> the headers, thus users such as the gallium radeon winsys has ended up
>> redefining/duplicating internally.
>
> The problem is that you generally need the latest headers to support
> the latest features in the user mode stacks, so if the user tries to
> build against an older kernel, they will be missing definitions.
>
Pretty much that's why we have the loose release criteria for libdrm.
Get the functionality upstream (mainline or -next), pull the changes
(sync) and release. As the sync has been busted for a while, people
are forced to do unpleasant things.

-Emil
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 04/79] drm_mode.h: use __u32 and __u64 from linux/types.h

[Mikko Rapeli]

On Thu, Oct 15, 2015 at 09:32:10AM -0400, Alex Deucher wrote:
> On Thu, Oct 15, 2015 at 1:55 AM, Mikko Rapeli  wrote:
> > Fixes userspace compilation error:
> >
> > drm/drm_mode.h:472:2: error: unknown type name ‘uint32_t’
> >
> > Signed-off-by: Mikko Rapeli 
> 
> NACK on all these type conversions.  This has not been a problem for
> years and years and the result looks terrible.

Documentation/CodingStyle, section 5

 (e) Types safe for use in userspace.

 In certain structures which are visible to userspace, we cannot
 require C99 types and cannot use the 'u32' form above. Thus, we
 use __u32 and similar types in all structures which are shared
 with userspace.

I have only been looking at kernel headers from userspace occationally in
the past 10 years and had a several cases where the provided headers did
not compile when included into trivial programs trying to use the structs
for an ioctl() for example. This long lasting problem triggered me to write
a test for this and provide these fixes too. In previous reviews usage
of  and its types in kernel headers was already NACK'ed
so I changed several places from uint32_t's to __u32.

With these changes it is btw trivial now to add a grep test the there
are no uint32_t's in include/uapi/ anymore, thus enforcing that coding style
rule.

-Mikko

> Alex
> 
> > ---
> >  include/uapi/drm/drm_mode.h | 16 
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index 359107a..0ed8d9d 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -508,14 +508,14 @@ struct drm_mode_crtc_page_flip {
> >
> >  /* create a dumb scanout buffer */
> >  struct drm_mode_create_dumb {
> > -   uint32_t height;
> > -   uint32_t width;
> > -   uint32_t bpp;
> > -   uint32_t flags;
> > +   __u32 height;
> > +   __u32 width;
> > +   __u32 bpp;
> > +   __u32 flags;
> > /* handle, pitch, size will be returned */
> > -   uint32_t handle;
> > -   uint32_t pitch;
> > -   uint64_t size;
> > +   __u32 handle;
> > +   __u32 pitch;
> > +   __u64 size;
> >  };
> >
> >  /* set up for mmap of a dumb scanout buffer */
> > @@ -532,7 +532,7 @@ struct drm_mode_map_dumb {
> >  };
> >
> >  struct drm_mode_destroy_dumb {
> > -   uint32_t handle;
> > +   __u32 handle;
> >  };
> >
> >  /* page-flip flags are valid, plus: */
> > --
> > 2.5.0
> >
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 04/79] drm_mode.h: use __u32 and __u64 from linux/types.h

[Alex Deucher]

On Thu, Oct 15, 2015 at 1:55 AM, Mikko Rapeli  wrote:
> Fixes userspace compilation error:
>
> drm/drm_mode.h:472:2: error: unknown type name ‘uint32_t’
>
> Signed-off-by: Mikko Rapeli 

NACK on all these type conversions.  This has not been a problem for
years and years and the result looks terrible.

Alex

> ---
>  include/uapi/drm/drm_mode.h | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 359107a..0ed8d9d 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -508,14 +508,14 @@ struct drm_mode_crtc_page_flip {
>
>  /* create a dumb scanout buffer */
>  struct drm_mode_create_dumb {
> -   uint32_t height;
> -   uint32_t width;
> -   uint32_t bpp;
> -   uint32_t flags;
> +   __u32 height;
> +   __u32 width;
> +   __u32 bpp;
> +   __u32 flags;
> /* handle, pitch, size will be returned */
> -   uint32_t handle;
> -   uint32_t pitch;
> -   uint64_t size;
> +   __u32 handle;
> +   __u32 pitch;
> +   __u64 size;
>  };
>
>  /* set up for mmap of a dumb scanout buffer */
> @@ -532,7 +532,7 @@ struct drm_mode_map_dumb {
>  };
>
>  struct drm_mode_destroy_dumb {
> -   uint32_t handle;
> +   __u32 handle;
>  };
>
>  /* page-flip flags are valid, plus: */
> --
> 2.5.0
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 04/79] drm_mode.h: use __u32 and __u64 from linux/types.h

[Mikko Rapeli]

On Thu, Oct 15, 2015 at 09:32:10AM -0400, Alex Deucher wrote:
> On Thu, Oct 15, 2015 at 1:55 AM, Mikko Rapeli  wrote:
> > Fixes userspace compilation error:
> >
> > drm/drm_mode.h:472:2: error: unknown type name ‘uint32_t’
> >
> > Signed-off-by: Mikko Rapeli 
> 
> NACK on all these type conversions.  This has not been a problem for
> years and years and the result looks terrible.

Documentation/CodingStyle, section 5

 (e) Types safe for use in userspace.

 In certain structures which are visible to userspace, we cannot
 require C99 types and cannot use the 'u32' form above. Thus, we
 use __u32 and similar types in all structures which are shared
 with userspace.

I have only been looking at kernel headers from userspace occationally in
the past 10 years and had a several cases where the provided headers did
not compile when included into trivial programs trying to use the structs
for an ioctl() for example. This long lasting problem triggered me to write
a test for this and provide these fixes too. In previous reviews usage
of  and its types in kernel headers was already NACK'ed
so I changed several places from uint32_t's to __u32.

With these changes it is btw trivial now to add a grep test the there
are no uint32_t's in include/uapi/ anymore, thus enforcing that coding style
rule.

-Mikko

> Alex
> 
> > ---
> >  include/uapi/drm/drm_mode.h | 16 
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index 359107a..0ed8d9d 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -508,14 +508,14 @@ struct drm_mode_crtc_page_flip {
> >
> >  /* create a dumb scanout buffer */
> >  struct drm_mode_create_dumb {
> > -   uint32_t height;
> > -   uint32_t width;
> > -   uint32_t bpp;
> > -   uint32_t flags;
> > +   __u32 height;
> > +   __u32 width;
> > +   __u32 bpp;
> > +   __u32 flags;
> > /* handle, pitch, size will be returned */
> > -   uint32_t handle;
> > -   uint32_t pitch;
> > -   uint64_t size;
> > +   __u32 handle;
> > +   __u32 pitch;
> > +   __u64 size;
> >  };
> >
> >  /* set up for mmap of a dumb scanout buffer */
> > @@ -532,7 +532,7 @@ struct drm_mode_map_dumb {
> >  };
> >
> >  struct drm_mode_destroy_dumb {
> > -   uint32_t handle;
> > +   __u32 handle;
> >  };
> >
> >  /* page-flip flags are valid, plus: */
> > --
> > 2.5.0
> >
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 04/79] drm_mode.h: use __u32 and __u64 from linux/types.h

[Alex Deucher]

On Thu, Oct 15, 2015 at 1:55 AM, Mikko Rapeli  wrote:
> Fixes userspace compilation error:
>
> drm/drm_mode.h:472:2: error: unknown type name ‘uint32_t’
>
> Signed-off-by: Mikko Rapeli 

NACK on all these type conversions.  This has not been a problem for
years and years and the result looks terrible.

Alex

> ---
>  include/uapi/drm/drm_mode.h | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 359107a..0ed8d9d 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -508,14 +508,14 @@ struct drm_mode_crtc_page_flip {
>
>  /* create a dumb scanout buffer */
>  struct drm_mode_create_dumb {
> -   uint32_t height;
> -   uint32_t width;
> -   uint32_t bpp;
> -   uint32_t flags;
> +   __u32 height;
> +   __u32 width;
> +   __u32 bpp;
> +   __u32 flags;
> /* handle, pitch, size will be returned */
> -   uint32_t handle;
> -   uint32_t pitch;
> -   uint64_t size;
> +   __u32 handle;
> +   __u32 pitch;
> +   __u64 size;
>  };
>
>  /* set up for mmap of a dumb scanout buffer */
> @@ -532,7 +532,7 @@ struct drm_mode_map_dumb {
>  };
>
>  struct drm_mode_destroy_dumb {
> -   uint32_t handle;
> +   __u32 handle;
>  };
>
>  /* page-flip flags are valid, plus: */
> --
> 2.5.0
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v4 04/79] drm_mode.h: use __u32 and __u64 from linux/types.h

[Mikko Rapeli]

Fixes userspace compilation error:

drm/drm_mode.h:472:2: error: unknown type name ‘uint32_t’

Signed-off-by: Mikko Rapeli 
---
 include/uapi/drm/drm_mode.h | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 359107a..0ed8d9d 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -508,14 +508,14 @@ struct drm_mode_crtc_page_flip {
 
 /* create a dumb scanout buffer */
 struct drm_mode_create_dumb {
-   uint32_t height;
-   uint32_t width;
-   uint32_t bpp;
-   uint32_t flags;
+   __u32 height;
+   __u32 width;
+   __u32 bpp;
+   __u32 flags;
/* handle, pitch, size will be returned */
-   uint32_t handle;
-   uint32_t pitch;
-   uint64_t size;
+   __u32 handle;
+   __u32 pitch;
+   __u64 size;
 };
 
 /* set up for mmap of a dumb scanout buffer */
@@ -532,7 +532,7 @@ struct drm_mode_map_dumb {
 };
 
 struct drm_mode_destroy_dumb {
-   uint32_t handle;
+   __u32 handle;
 };
 
 /* page-flip flags are valid, plus: */
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v4 04/79] drm_mode.h: use __u32 and __u64 from linux/types.h

[Mikko Rapeli]

Fixes userspace compilation error:

drm/drm_mode.h:472:2: error: unknown type name ‘uint32_t’

Signed-off-by: Mikko Rapeli 
---
 include/uapi/drm/drm_mode.h | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 359107a..0ed8d9d 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -508,14 +508,14 @@ struct drm_mode_crtc_page_flip {
 
 /* create a dumb scanout buffer */
 struct drm_mode_create_dumb {
-   uint32_t height;
-   uint32_t width;
-   uint32_t bpp;
-   uint32_t flags;
+   __u32 height;
+   __u32 width;
+   __u32 bpp;
+   __u32 flags;
/* handle, pitch, size will be returned */
-   uint32_t handle;
-   uint32_t pitch;
-   uint64_t size;
+   __u32 handle;
+   __u32 pitch;
+   __u64 size;
 };
 
 /* set up for mmap of a dumb scanout buffer */
@@ -532,7 +532,7 @@ struct drm_mode_map_dumb {
 };
 
 struct drm_mode_destroy_dumb {
-   uint32_t handle;
+   __u32 handle;
 };
 
 /* page-flip flags are valid, plus: */
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/