On Mon, Mar 15, 2021 at 05:54:17PM +0000, Alex Bennée wrote: > > Daniel P. Berrangé <berra...@redhat.com> writes: > > > On Mon, Mar 15, 2021 at 06:04:10PM +0100, Thomas Huth wrote: > >> On 15/03/2021 17.57, Peter Maydell wrote: > >> > On Mon, 15 Mar 2021 at 16:53, Alex Bennée <alex.ben...@linaro.org> wrote: > >> > > -Prefer g_new(T, n) instead of g_malloc(sizeof(T) ``*`` n) for the > >> > > following > >> > > +Care should be taken to avoid introducing places where the guest could > >> > > +trigger an exit. For example using ``g_malloc`` on start-up is fine > >> > > +if the result of a failure is going to be a fatal exit anyway. There > >> > > +may be some start-up cases where failing is unreasonable (for example > >> > > +speculatively loading debug symbols). > >> > > + > >> > > +However if we are doing an allocation because of something the guest > >> > > +has done we should never trigger an exit. The code may deal with this > >> > > +by trying to allocate less memory and continue or re-designed to > >> > > allocate > >> > > +buffers on start-up. > >> > > >> > I think this is overly strong. We want to avoid malloc-or-die for > >> > cases where the guest gets to decide how big the allocation is; > >> > but if we're doing a single small fixed-size allocation that happens > >> > to be triggered by a guest action we should be OK to g_malloc() that > >> > I think. > >> > >> I agree with Peter. If the host is so much out-of-memory that we even can't > >> allocate some few bytes anymore (let's say less than 4k), the system is > >> pretty much dead anyway and it might be better to terminate the program > >> immediately instead of continuing with the out-of-memory situation. > > > > On a Linux host you're almost certainly not going to see g_malloc > > fail for small allocations at least. Instead at some point the host > > will be under enough memory pressure that the OOM killer activates > > and reaps arbitrary processes based on some criteria it has, freeing > > up memory for malloc to succeed (unless OOM killer picked you as the > > victim). > > OK how about this wording: > > Please note that ``g_malloc`` will exit on allocation failure, so > there is no need to test for failure (as you would have to with > ``malloc``). Generally using ``g_malloc`` on start-up is fine as the > result of a failure to allocate memory is going to be a fatal exit > anyway. There may be some start-up cases where failing is unreasonable > (for example speculatively loading a large debug symbol table). > > Care should be taken to avoid introducing places where the guest could > trigger an exit by causing a large allocation. For small allocations, > of the order of 4k, a failure to allocate is likely indicative of an > overloaded host and allowing ``g_malloc`` to ``exit`` is a reasonable > approach. However for larger allocations where we could realistically > fall-back to a smaller one if need be we should use functions like > ``g_try_new`` and check the result. For example this is valid approach > for a time/space trade-off like ``tlb_mmu_resize_locked`` in the > SoftMMU TLB code.
Fine with me Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
