[PATCH] scatterlist: add a warning if sg_virt() is used on highmem pages

[PATCH] scatterlist: add a warning if sg_virt() is used on highmem pages

[Sebastian Andrzej Siewior]

sg_virt() on highmem pages won't work. This WARN_ON() should catch some
that still try.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/scatterlist.h |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index 4bd6c06..4d4adab 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -198,6 +198,9 @@ static inline dma_addr_t sg_phys(struct scatterlist *sg)
  **/
 static inline void *sg_virt(struct scatterlist *sg)
 {
+#ifdef CONFIG_DEBUG_SG
+	WARN_ON(PageHighMem(sg_page(sg)));
+#endif
 	return page_address(sg_page(sg)) + sg->offset;
 }
 
-- 
1.7.10.4

Re: [PATCH] scatterlist: add a warning if sg_virt() is used on highmem pages

[Andrew Morton]

On Wed,  5 Dec 2012 12:28:18 +0100
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> sg_virt() on highmem pages won't work. This WARN_ON() should catch some
> that still try.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  include/linux/scatterlist.h |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
> index 4bd6c06..4d4adab 100644
> --- a/include/linux/scatterlist.h
> +++ b/include/linux/scatterlist.h
> @@ -198,6 +198,9 @@ static inline dma_addr_t sg_phys(struct scatterlist *sg)
>   **/
>  static inline void *sg_virt(struct scatterlist *sg)
>  {
> +#ifdef CONFIG_DEBUG_SG
> +	WARN_ON(PageHighMem(sg_page(sg)));
> +#endif
>  	return page_address(sg_page(sg)) + sg->offset;
>  }

Why won't it work?  page_address() will search the kmap table and will
search for the page.  If the caller had previously kmapped that page,
all is well.  If the caller has failed to kmap the page, sg_virt() will
return a nearly-null pointer and presumably someone will later go oops.

Re: [PATCH] scatterlist: add a warning if sg_virt() is used on highmem pages

[Sebastian Andrzej Siewior]

On 12/05/2012 10:46 PM, Andrew Morton wrote:
>> --- a/include/linux/scatterlist.h
>> +++ b/include/linux/scatterlist.h
>> @@ -198,6 +198,9 @@ static inline dma_addr_t sg_phys(struct scatterlist *sg)
>>    **/
>>   static inline void *sg_virt(struct scatterlist *sg)
>>   {
>> +#ifdef CONFIG_DEBUG_SG
>> +	WARN_ON(PageHighMem(sg_page(sg)));
>> +#endif
>>   	return page_address(sg_page(sg)) + sg->offset;
>>   }
>
> Why won't it work?  page_address() will search the kmap table and will
> search for the page.  If the caller had previously kmapped that page,
> all is well.  If the caller has failed to kmap the page, sg_virt() will
> return a nearly-null pointer and presumably someone will later go oops.

I missed that it will work for previously mapped page. If you think
that an oops later is enough then okay, I drop this.

Sebastian