diff mbox

[RFC,24/25] drm/i915: Zero fill the request structure

Message ID 1412941272-6350-1-git-send-email-John.C.Harrison@Intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

John Harrison Oct. 10, 2014, 11:41 a.m. UTC 
From: John Harrison <John.C.Harrison@Intel.com>

For: VIZ-4377
Signed-off-by: John.C.Harrison@Intel.com
---
 drivers/gpu/drm/i915/intel_lrc.c        |    3 +--
 drivers/gpu/drm/i915/intel_ringbuffer.c |    3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

Comments

Daniel Vetter Oct. 19, 2014, 2:15 p.m. UTC | #1 
On Fri, Oct 10, 2014 at 12:41:12PM +0100, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> For: VIZ-4377
> Signed-off-by: John.C.Harrison@Intel.com

I think this should be squashed (well, split first) into the relevant
earlier patches. Generally I much prefer kzalloc, and we use that almost
everywhere.

Or does this silently fix some issue and doesn't tell?
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_lrc.c        |    3 +--
>  drivers/gpu/drm/i915/intel_ringbuffer.c |    3 +--
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 97e6b92..5a75eb5 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -802,13 +802,12 @@ static int logical_ring_alloc_request(struct intel_engine_cs *ring,
>  	if (ring->outstanding_lazy_request)
>  		return 0;
>  
> -	request = kmalloc(sizeof(*request), GFP_KERNEL);
> +	request = kzalloc(sizeof(*request), GFP_KERNEL);
>  	if (request == NULL)
>  		return -ENOMEM;
>  
>  	kref_init(&request->ref);
>  	request->ring = ring;
> -	request->complete = false;
>  
>  	ret = i915_gem_get_seqno(ring->dev, &request->seqno);
>  	if (ret) {
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index df9c7e3..0f2719d 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -2017,13 +2017,12 @@ intel_ring_alloc_request(struct intel_engine_cs *ring)
>  	if (ring->outstanding_lazy_request)
>  		return 0;
>  
> -	request = kmalloc(sizeof(*request), GFP_KERNEL);
> +	request = kzalloc(sizeof(*request), GFP_KERNEL);
>  	if (request == NULL)
>  		return -ENOMEM;
>  
>  	kref_init(&request->ref);
>  	request->ring = ring;
> -	request->complete = false;
>  
>  	ret = i915_gem_get_seqno(ring->dev, &request->seqno);
>  	if (ret) {
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
John Harrison Oct. 28, 2014, 3:55 p.m. UTC | #2 
On 19/10/2014 15:15, Daniel Vetter wrote:
> On Fri, Oct 10, 2014 at 12:41:12PM +0100, John.C.Harrison@Intel.com wrote:
>> From: John Harrison <John.C.Harrison@Intel.com>
>>
>> For: VIZ-4377
>> Signed-off-by: John.C.Harrison@Intel.com
> I think this should be squashed (well, split first) into the relevant
> earlier patches. Generally I much prefer kzalloc, and we use that almost
> everywhere.
>
> Or does this silently fix some issue and doesn't tell?
> -Daniel
No, I just left it separate so as to change as little as possible in 
each individual patch. The original code did a kmalloc() of the request. 
That predates my changes. So if I did a km -> kz change in the middle of 
some other work I was assuming you would complain at me doing too much 
in a single patch. Whereas, the 'complete' field is the first time a 
zero fill becomes significant so it made sense (to me) to leave the 
switch to kz until here.


>> ---
>>   drivers/gpu/drm/i915/intel_lrc.c        |    3 +--
>>   drivers/gpu/drm/i915/intel_ringbuffer.c |    3 +--
>>   2 files changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>> index 97e6b92..5a75eb5 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> @@ -802,13 +802,12 @@ static int logical_ring_alloc_request(struct intel_engine_cs *ring,
>>   	if (ring->outstanding_lazy_request)
>>   		return 0;
>>   
>> -	request = kmalloc(sizeof(*request), GFP_KERNEL);
>> +	request = kzalloc(sizeof(*request), GFP_KERNEL);
>>   	if (request == NULL)
>>   		return -ENOMEM;
>>   
>>   	kref_init(&request->ref);
>>   	request->ring = ring;
>> -	request->complete = false;
>>   
>>   	ret = i915_gem_get_seqno(ring->dev, &request->seqno);
>>   	if (ret) {
>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> index df9c7e3..0f2719d 100644
>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> @@ -2017,13 +2017,12 @@ intel_ring_alloc_request(struct intel_engine_cs *ring)
>>   	if (ring->outstanding_lazy_request)
>>   		return 0;
>>   
>> -	request = kmalloc(sizeof(*request), GFP_KERNEL);
>> +	request = kzalloc(sizeof(*request), GFP_KERNEL);
>>   	if (request == NULL)
>>   		return -ENOMEM;
>>   
>>   	kref_init(&request->ref);
>>   	request->ring = ring;
>> -	request->complete = false;
>>   
>>   	ret = i915_gem_get_seqno(ring->dev, &request->seqno);
>>   	if (ret) {
>> -- 
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Daniel Vetter Nov. 3, 2014, 11:02 a.m. UTC | #3 
On Tue, Oct 28, 2014 at 03:55:21PM +0000, John Harrison wrote:
> On 19/10/2014 15:15, Daniel Vetter wrote:
> >On Fri, Oct 10, 2014 at 12:41:12PM +0100, John.C.Harrison@Intel.com wrote:
> >>From: John Harrison <John.C.Harrison@Intel.com>
> >>
> >>For: VIZ-4377
> >>Signed-off-by: John.C.Harrison@Intel.com
> >I think this should be squashed (well, split first) into the relevant
> >earlier patches. Generally I much prefer kzalloc, and we use that almost
> >everywhere.
> >
> >Or does this silently fix some issue and doesn't tell?
> >-Daniel
> No, I just left it separate so as to change as little as possible in each
> individual patch. The original code did a kmalloc() of the request. That
> predates my changes. So if I did a km -> kz change in the middle of some
> other work I was assuming you would complain at me doing too much in a
> single patch. Whereas, the 'complete' field is the first time a zero fill
> becomes significant so it made sense (to me) to leave the switch to kz until
> here.

Makes sense, but this kind of explanation really should be in the commit
message.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 97e6b92..5a75eb5 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -802,13 +802,12 @@  static int logical_ring_alloc_request(struct intel_engine_cs *ring,
 	if (ring->outstanding_lazy_request)
 		return 0;
 
-	request = kmalloc(sizeof(*request), GFP_KERNEL);
+	request = kzalloc(sizeof(*request), GFP_KERNEL);
 	if (request == NULL)
 		return -ENOMEM;
 
 	kref_init(&request->ref);
 	request->ring = ring;
-	request->complete = false;
 
 	ret = i915_gem_get_seqno(ring->dev, &request->seqno);
 	if (ret) {
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index df9c7e3..0f2719d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2017,13 +2017,12 @@  intel_ring_alloc_request(struct intel_engine_cs *ring)
 	if (ring->outstanding_lazy_request)
 		return 0;
 
-	request = kmalloc(sizeof(*request), GFP_KERNEL);
+	request = kzalloc(sizeof(*request), GFP_KERNEL);
 	if (request == NULL)
 		return -ENOMEM;
 
 	kref_init(&request->ref);
 	request->ring = ring;
-	request->complete = false;
 
 	ret = i915_gem_get_seqno(ring->dev, &request->seqno);
 	if (ret) {