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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxx>                              
>    >Signed-off-by: Nirmoy Das [3]<nirmoy.das@xxxxxxxxx>                         
>    >---                                                                         
>    > 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@xxxxxxxxx>

BTW, what email clients are you using recently?
it is hard to parse your responses lately. Please check if it is really
sending/replying as text-only mode.

>                                                                                 
>    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@xxxxxxxxxxxxxxx
>    3. mailto:nirmoy.das@xxxxxxxxx



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux