Re: [Lsf-pc] [LSF/MM/BPF TOPIC] Reclamation interactions with RCU

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

 



On Fri, 22 Mar 2024, Dan Carpenter wrote:
> On Fri, Mar 22, 2024 at 12:47:42PM +1100, NeilBrown wrote:
> > On Thu, 21 Mar 2024, Dan Carpenter wrote:
> > > On Mon, Mar 04, 2024 at 09:45:48AM +1100, NeilBrown wrote:
> > > > I have in mind a more explicit statement of how much waiting is
> > > > acceptable.
> > > > 
> > > > GFP_NOFAIL - wait indefinitely
> > > 
> > > Why not call it GFP_SMALL?  It wouldn't fail.  The size would have to be
> > > less than some limit.  If the size was too large, that would trigger a
> > > WARN_ON_ONCE().
> > 
> > I would be happy with GFP_SMALL.  It would never return NULL but might
> > block indefinitely.  It would (as you say) WARN (maybe ONCE) if the size
> > was considered "COSTLY" and would possibly BUG if the size exceeded
> > KMALLOC_MAX_SIZE. 
> 
> I'd like to keep GFP_SMALL much smaller than KMALLOC_MAX_SIZE.  IIf
> you're allocating larger than that, you'd still be able to GFP_NOFAIL.
> I looked quickly an I think over 60% of allocations are just sizeof(*p)
> and probably 90% are under 4k.

What do you mean exactly by "keep"??
Do you mean WARN_ON if it is "too big" - certainly agree.
Do you mean BUG_ON if it is "too big" - maybe agree.
Do you mean return NULL if it is "too big" - definitely disagree.
Do you mean build failure if it could be too big - I would LOVE that,
but I don't think we can do that with current build tools.

Thanks,
NeilBrown



> 
> > 
> > > 
> > > I obviously understand that this duplicates the information in the size
> > > parameter but the point is that GFP_SMALL allocations have been
> > > reviewed, updated, and don't have error handling code.
> > 
> > We are on the same page here.
> > 
> > > 
> > > We'd keep GFP_KERNEL which would keep the existing behavior.  (Which is
> > > that it can sleep and it can fail).  I think that maps to GFP_RETRY but
> > > GFP_RETRY is an uglier name.
> > 
> > Can it fail though?  I know it is allowed to, but does it happen?
> > 
> 
> In some sense, I don't really care about this, I just want the rules
> clear from a static analysis perspective.  Btw, you're discussing making
> the too small to fail rule official but other times we have discussed
> getting rid of it all together.  So I think maybe it's better to keep
> the rules strict but allow the actual implentation to change later.
> 
> The GFP_SMALL stuff is nice for static analysis because it would warn
> about anything larger than whatever the small limit is.  So that means I
> have fewer allocations to review for integer overflow bugs.
> 
> Btw, Jiri Pirko, was proposing a macro which would automatically
> allocate the 60+% of allocations which are sizeof(*p).
> https://lore.kernel.org/all/20240315132249.2515468-1-jiri@xxxxxxxxxxx/
> I had offered an alternative macro but the idea is the same:
> 
> #define __ALLOC(p) p __free(kfree) = kzalloc(sizeof(*p), GFP_KERNEL)
>         struct ice_aqc_get_phy_caps_data *__ALLOC(pcaps);
> 
> Combining no fail allocations with automatic cleanup changes the way you
> write code.
> 
> And then on the success patch you have the no_free_ptr() which I haven't
> used but I think will be so useful for static analysis.  I'm so excited
> about this.
> 
> regards,
> dan carpenter
> 
> 
> 





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux