linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] agp: fix scratch page cleanup
@ 2012-01-25 15:51 Michal Kubecek
  2012-01-25 19:01 ` Dave Airlie
  0 siblings, 1 reply; 3+ messages in thread
From: Michal Kubecek @ 2012-01-25 15:51 UTC (permalink / raw)
  To: David Airlie; +Cc: linux-kernel

In error cleanup of agp_backend_initialize() and in agp_backend_cleanup(),
agp_destroy_page() is passed virtual address of the scratch page. This
leads to a kernel warning if the initialization fails (or upon regular
cleanup) as pointer to struct page should be passed instead.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 drivers/char/agp/backend.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/char/agp/backend.c b/drivers/char/agp/backend.c
index 4b71647..317c28c 100644
--- a/drivers/char/agp/backend.c
+++ b/drivers/char/agp/backend.c
@@ -194,10 +194,10 @@ static int agp_backend_initialize(struct agp_bridge_data *bridge)
 
 err_out:
 	if (bridge->driver->needs_scratch_page) {
-		void *va = page_address(bridge->scratch_page_page);
+		struct page *page = bridge->scratch_page_page;
 
-		bridge->driver->agp_destroy_page(va, AGP_PAGE_DESTROY_UNMAP);
-		bridge->driver->agp_destroy_page(va, AGP_PAGE_DESTROY_FREE);
+		bridge->driver->agp_destroy_page(page, AGP_PAGE_DESTROY_UNMAP);
+		bridge->driver->agp_destroy_page(page, AGP_PAGE_DESTROY_FREE);
 	}
 	if (got_gatt)
 		bridge->driver->free_gatt_table(bridge);
@@ -221,10 +221,10 @@ static void agp_backend_cleanup(struct agp_bridge_data *bridge)
 
 	if (bridge->driver->agp_destroy_page &&
 	    bridge->driver->needs_scratch_page) {
-		void *va = page_address(bridge->scratch_page_page);
+		struct page *page = bridge->scratch_page_page;
 
-		bridge->driver->agp_destroy_page(va, AGP_PAGE_DESTROY_UNMAP);
-		bridge->driver->agp_destroy_page(va, AGP_PAGE_DESTROY_FREE);
+		bridge->driver->agp_destroy_page(page, AGP_PAGE_DESTROY_UNMAP);
+		bridge->driver->agp_destroy_page(page, AGP_PAGE_DESTROY_FREE);
 	}
 }
 
-- 
1.7.7


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

* Re: [PATCH] agp: fix scratch page cleanup
  2012-01-25 15:51 [PATCH] agp: fix scratch page cleanup Michal Kubecek
@ 2012-01-25 19:01 ` Dave Airlie
  2012-01-26  9:45   ` Jan Beulich
  0 siblings, 1 reply; 3+ messages in thread
From: Dave Airlie @ 2012-01-25 19:01 UTC (permalink / raw)
  To: Michal Kubecek, JBeulich; +Cc: linux-kernel


> In error cleanup of agp_backend_initialize() and in agp_backend_cleanup(),
> agp_destroy_page() is passed virtual address of the scratch page. This
> leads to a kernel warning if the initialization fails (or upon regular
> cleanup) as pointer to struct page should be passed instead.

Jan can you check and ack this if okay? since you wrote this chunk 
originally?

Dave.

> 
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
> ---
>  drivers/char/agp/backend.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/char/agp/backend.c b/drivers/char/agp/backend.c
> index 4b71647..317c28c 100644
> --- a/drivers/char/agp/backend.c
> +++ b/drivers/char/agp/backend.c
> @@ -194,10 +194,10 @@ static int agp_backend_initialize(struct agp_bridge_data *bridge)
>  
>  err_out:
>  	if (bridge->driver->needs_scratch_page) {
> -		void *va = page_address(bridge->scratch_page_page);
> +		struct page *page = bridge->scratch_page_page;
>  
> -		bridge->driver->agp_destroy_page(va, AGP_PAGE_DESTROY_UNMAP);
> -		bridge->driver->agp_destroy_page(va, AGP_PAGE_DESTROY_FREE);
> +		bridge->driver->agp_destroy_page(page, AGP_PAGE_DESTROY_UNMAP);
> +		bridge->driver->agp_destroy_page(page, AGP_PAGE_DESTROY_FREE);
>  	}
>  	if (got_gatt)
>  		bridge->driver->free_gatt_table(bridge);
> @@ -221,10 +221,10 @@ static void agp_backend_cleanup(struct agp_bridge_data *bridge)
>  
>  	if (bridge->driver->agp_destroy_page &&
>  	    bridge->driver->needs_scratch_page) {
> -		void *va = page_address(bridge->scratch_page_page);
> +		struct page *page = bridge->scratch_page_page;
>  
> -		bridge->driver->agp_destroy_page(va, AGP_PAGE_DESTROY_UNMAP);
> -		bridge->driver->agp_destroy_page(va, AGP_PAGE_DESTROY_FREE);
> +		bridge->driver->agp_destroy_page(page, AGP_PAGE_DESTROY_UNMAP);
> +		bridge->driver->agp_destroy_page(page, AGP_PAGE_DESTROY_FREE);
>  	}
>  }
>  
> 

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

* Re: [PATCH] agp: fix scratch page cleanup
  2012-01-25 19:01 ` Dave Airlie
@ 2012-01-26  9:45   ` Jan Beulich
  0 siblings, 0 replies; 3+ messages in thread
From: Jan Beulich @ 2012-01-26  9:45 UTC (permalink / raw)
  To: Dave Airlie; +Cc: Michal Kubecek, linux-kernel

>>> On 25.01.12 at 20:01, Dave Airlie <airlied@linux.ie> wrote:

>> In error cleanup of agp_backend_initialize() and in agp_backend_cleanup(),
>> agp_destroy_page() is passed virtual address of the scratch page. This
>> leads to a kernel warning if the initialization fails (or upon regular
>> cleanup) as pointer to struct page should be passed instead.
> 
> Jan can you check and ack this if okay? since you wrote this chunk 
> originally?

The change here is fine, but rather fixes an omission of your own
"agp: switch AGP to use page array instead of unsigned long array"
(which went in for 2.6.31, whereas my change went into 2.6.26),
which also wasn't noticed when David Woodhouse touched the
code again for 2.6.32 ("agp: Switch agp_{un,}map_page() to take
struct page * argument").

Jan

>> 
>> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
>> ---
>>  drivers/char/agp/backend.c |   12 ++++++------
>>  1 files changed, 6 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/char/agp/backend.c b/drivers/char/agp/backend.c
>> index 4b71647..317c28c 100644
>> --- a/drivers/char/agp/backend.c
>> +++ b/drivers/char/agp/backend.c
>> @@ -194,10 +194,10 @@ static int agp_backend_initialize(struct 
> agp_bridge_data *bridge)
>>  
>>  err_out:
>>  	if (bridge->driver->needs_scratch_page) {
>> -		void *va = page_address(bridge->scratch_page_page);
>> +		struct page *page = bridge->scratch_page_page;
>>  
>> -		bridge->driver->agp_destroy_page(va, AGP_PAGE_DESTROY_UNMAP);
>> -		bridge->driver->agp_destroy_page(va, AGP_PAGE_DESTROY_FREE);
>> +		bridge->driver->agp_destroy_page(page, AGP_PAGE_DESTROY_UNMAP);
>> +		bridge->driver->agp_destroy_page(page, AGP_PAGE_DESTROY_FREE);
>>  	}
>>  	if (got_gatt)
>>  		bridge->driver->free_gatt_table(bridge);
>> @@ -221,10 +221,10 @@ static void agp_backend_cleanup(struct agp_bridge_data 
> *bridge)
>>  
>>  	if (bridge->driver->agp_destroy_page &&
>>  	    bridge->driver->needs_scratch_page) {
>> -		void *va = page_address(bridge->scratch_page_page);
>> +		struct page *page = bridge->scratch_page_page;
>>  
>> -		bridge->driver->agp_destroy_page(va, AGP_PAGE_DESTROY_UNMAP);
>> -		bridge->driver->agp_destroy_page(va, AGP_PAGE_DESTROY_FREE);
>> +		bridge->driver->agp_destroy_page(page, AGP_PAGE_DESTROY_UNMAP);
>> +		bridge->driver->agp_destroy_page(page, AGP_PAGE_DESTROY_FREE);
>>  	}
>>  }
>>  
>> 




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

end of thread, other threads:[~2012-01-26  9:44 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-25 15:51 [PATCH] agp: fix scratch page cleanup Michal Kubecek
2012-01-25 19:01 ` Dave Airlie
2012-01-26  9:45   ` Jan Beulich

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).