[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Fixes integer overflow or integer truncation issues in page lookups, ttm place configuration and scatterlist creation

[Gwan-gyeong Mun]

On 12/28/22 9:51 PM, Patchwork wrote:
> == Series Details ==
> 
> Series: Fixes integer overflow or integer truncation issues in page lookups, ttm place configuration and scatterlist creation
> URL   : https://patchwork.freedesktop.org/series/112279/
> State : warning
> 
> == Summary ==
> 
> Error: dim checkpatch failed
> 580bc7c6ee10 drm/i915/gem: Typecheck page lookups
> -:56: WARNING:DEPRECATED_API: Deprecated use of 'kmap_atomic', prefer 'kmap_local_page' instead
> #56: FILE: drivers/gpu/drm/i915/gem/i915_gem_object.c:434:
> +	src_map = kmap_atomic(i915_gem_object_get_page(obj, idx));
The kmap_atomic() used in this patch series is not a new addition, but 
the input argument used in the existing kmap_atomic() call is replaced 
with a local variable.
Therefore, I suggest the discussion of replacing kmap_atomic() with 
kmap_local_page() should be considered in a separate patch.
Unlike kmap_local_page(), kmap_atomic() is accompanied by additional 
operations (preempt_disable, pagefault_disable).
Therefore, it is necessary to separately review whether there is a side 
effect by changing kmap_atomic() to kmap_local_page().
(Note. In the current implementation on i915, only kmap_atomic() is used 
(used in 13 places) and kmap_local_page() is not used.)

[include/linux/highmem-internal.h]
static inline void *kmap_atomic(struct page *page)
{
	if (IS_ENABLED(CONFIG_PREEMPT_RT))
		migrate_disable();
	else
		preempt_disable();
	pagefault_disable();
	return page_address(page);
}
...
static inline void *kmap_local_page(struct page *page)
{
	return page_address(page);
}
> 
> -:76: WARNING:AVOID_BUG: Do not crash the kernel unless it is absolutely unavoidable--use WARN_ON_ONCE() plus recovery code (if feasible) instead of BUG() or variants
> #76: FILE: drivers/gpu/drm/i915/gem/i915_gem_object.c:489:
> +	GEM_BUG_ON(overflows_type(offset >> PAGE_SHIFT, pgoff_t));
GEM_BUG_ON() used in this patch series is not a new addition, but the 
macro of the argument used for input has been changed from the 
previously used GEM_BUG_ON().
Changing GEM_BUG_ON() to a recoverable code should be considered in a 
separate patch.

Br,
G.G.
> 
> -:150: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'n' - possible side-effects?
> #150: FILE: drivers/gpu/drm/i915/gem/i915_gem_object.h:413:
> +#define i915_gem_object_page_iter_get_sg(obj, it, n, offset) ({	\
> +	static_assert(castable_to_type(n, pgoff_t));		\
> +	__i915_gem_object_page_iter_get_sg(obj, it, n, offset);	\
> +})
> 
> -:199: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'n' - possible side-effects?
> #199: FILE: drivers/gpu/drm/i915/gem/i915_gem_object.h:458:
> +#define i915_gem_object_get_sg(obj, n, offset) ({	\
> +	static_assert(castable_to_type(n, pgoff_t));	\
> +	__i915_gem_object_get_sg(obj, n, offset);	\
> +})
> 
> -:248: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'n' - possible side-effects?
> #248: FILE: drivers/gpu/drm/i915/gem/i915_gem_object.h:503:
> +#define i915_gem_object_get_sg_dma(obj, n, offset) ({	\
> +	static_assert(castable_to_type(n, pgoff_t));	\
> +	__i915_gem_object_get_sg_dma(obj, n, offset);	\
> +})
> 
> -:286: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'n' - possible side-effects?
> #286: FILE: drivers/gpu/drm/i915/gem/i915_gem_object.h:539:
> +#define i915_gem_object_get_page(obj, n) ({		\
> +	static_assert(castable_to_type(n, pgoff_t));	\
> +	__i915_gem_object_get_page(obj, n);		\
> +})
> 
> -:323: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'n' - possible side-effects?
> #323: FILE: drivers/gpu/drm/i915/gem/i915_gem_object.h:574:
> +#define i915_gem_object_get_dirty_page(obj, n) ({	\
> +	static_assert(castable_to_type(n, pgoff_t));	\
> +	__i915_gem_object_get_dirty_page(obj, n);	\
> +})
> 
> -:364: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'n' - possible side-effects?
> #364: FILE: drivers/gpu/drm/i915/gem/i915_gem_object.h:612:
> +#define i915_gem_object_get_dma_address_len(obj, n, len) ({	\
> +	static_assert(castable_to_type(n, pgoff_t));		\
> +	__i915_gem_object_get_dma_address_len(obj, n, len);	\
> +})
> 
> -:401: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'n' - possible side-effects?
> #401: FILE: drivers/gpu/drm/i915/gem/i915_gem_object.h:647:
> +#define i915_gem_object_get_dma_address(obj, n) ({	\
> +	static_assert(castable_to_type(n, pgoff_t));	\
> +	__i915_gem_object_get_dma_address(obj, n);	\
> +})
> 
> total: 0 errors, 2 warnings, 7 checks, 616 lines checked
> 383085856287 drm/i915: Check for integer truncation on scatterlist creation
> 60d38f11dfc7 drm/i915: Check for integer truncation on the configuration of ttm place
> c51e58da471c drm/i915: Check if the size is too big while creating shmem file
> 96ee63399a5e drm/i915: Use error code as -E2BIG when the size of gem ttm object is too large
> 2402a45e5aac drm/i915: Remove truncation warning for large objects
> 
>