[PATCH] drm/i915/gem: Suppress oom warning in favour of ENOMEM to userspace

[Nirmoy Das]

Hi Rodrigo,

On 6/26/2024 5:24 PM, Rodrigo Vivi wrote:
> On Wed, Jun 26, 2024 at 04:33:18PM +0200, Nirmoy Das wrote:
>> We report object allocation failures to userspace with ENOMEM
>> so add __GFP_NOWARN to remove superfluous oom warnings.
>>
>> Closes:https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4936
>> Cc: Andi Shyti<andi.shyti at linux.intel.com>
>> Signed-off-by: Nirmoy Das<nirmoy.das at intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_scatterlist.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_scatterlist.c b/drivers/gpu/drm/i915/i915_scatterlist.c
>> index e93d2538f298..4d830740946d 100644
>> --- a/drivers/gpu/drm/i915/i915_scatterlist.c
>> +++ b/drivers/gpu/drm/i915/i915_scatterlist.c
>> @@ -90,7 +90,7 @@ struct i915_refct_sgt *i915_rsgt_from_mm_node(const struct drm_mm_node *node,
>>   
>>   	GEM_BUG_ON(!max_segment);
>>   
>> -	rsgt = kmalloc(sizeof(*rsgt), GFP_KERNEL);
>> +	rsgt = kmalloc(sizeof(*rsgt), GFP_KERNEL | __GFP_NOWARN);
>>   	if (!rsgt)
>>   		return ERR_PTR(-ENOMEM);
> is it really safe?
> I don't believe we can guarantee a good fallback plan here if allocation fails.
> __i915_refct_sgt_init
> might end up in a null dereference, no?!

Kernel is now returning ENOMEM and also throwing a oom warning stack. 
With __GFP_NOWARN

the oom warning stack won't be there in the dmesg but userspace will 
still get ENOMEM as expected.

Let me know if got your question correctly.

Regards,

Nirmoy

>
>>   
>> @@ -104,7 +104,7 @@ struct i915_refct_sgt *i915_rsgt_from_mm_node(const struct drm_mm_node *node,
>>   	}
>>   
>>   	if (sg_alloc_table(st, DIV_ROUND_UP_ULL(node->size, segment_pages),
>> -			   GFP_KERNEL)) {
>> +			   GFP_KERNEL | __GFP_NOWARN)) {
>>   		i915_refct_sgt_put(rsgt);
>>   		return ERR_PTR(-ENOMEM);
>>   	}
>> @@ -178,7 +178,7 @@ struct i915_refct_sgt *i915_rsgt_from_buddy_resource(struct ttm_resource *res,
>>   	GEM_BUG_ON(list_empty(blocks));
>>   	GEM_BUG_ON(!max_segment);
>>   
>> -	rsgt = kmalloc(sizeof(*rsgt), GFP_KERNEL);
>> +	rsgt = kmalloc(sizeof(*rsgt), GFP_KERNEL | __GFP_NOWARN);
>>   	if (!rsgt)
>>   		return ERR_PTR(-ENOMEM);
>>   
>> @@ -190,7 +190,7 @@ struct i915_refct_sgt *i915_rsgt_from_buddy_resource(struct ttm_resource *res,
>>   		return ERR_PTR(-E2BIG);
>>   	}
>>   
>> -	if (sg_alloc_table(st, PFN_UP(res->size), GFP_KERNEL)) {
>> +	if (sg_alloc_table(st, PFN_UP(res->size), GFP_KERNEL | __GFP_NOWARN)) {
>>   		i915_refct_sgt_put(rsgt);
>>   		return ERR_PTR(-ENOMEM);
>>   	}
>> -- 
>> 2.42.0
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20240626/c7717d7a/attachment-0001.htm>