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

[Nirmoy Das]

Hi Rodrigo,

On 6/26/2024 5:50 PM, Rodrigo Vivi wrote:
> On Wed, Jun 26, 2024 at 05:36:43PM +0200, Nirmoy Das wrote:
>>     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: [1]https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4936
>>     >Cc: Andi Shyti [2]<andi.shyti at linux.intel.com>
>>     >Signed-off-by: Nirmoy Das [3]<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.
> doh! I had missunderstand the flag. Thanks for the confirmation.
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>
> BTW, what email clients are you using recently?

Using the same client, Thunderbird.


> it is hard to parse your responses lately. Please check if it is really
> sending/replying as text-only mode.

Thanks for notifying me. May be recent update changed some settings. I 
will check.


Nirmoy

>
>>                                                                                  
>>     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
>>
>> References
>>
>>     Visible links
>>     1. https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4936
>>     2. mailto:andi.shyti at linux.intel.com
>>     3. mailto:nirmoy.das at intel.com