Re: Temporary memory allocation from interrupt context
On Thu, Nov 12, 2020 at 10:51:58AM +0100, Lars Reichardt wrote: > I was/am under the impression that the longtime goal is to move all > allocations out of interrupt paths and that kmem_intr_alloc is there > only for transition, I have doubts this will happen. That was my impression as well, so I am very reluctant to introduce new uses of kmem_intr_alloc. I am currently investigating options to rearange the code at hand to completley move all allocations to thread context. Martin
Re: Temporary memory allocation from interrupt context
On Wed, 11 Nov 2020 07:32:56 -0800 Jason Thorpe wrote: > > On Nov 11, 2020, at 5:38 AM, Martin Husemann > > wrote: > > > > Yes, and of course the real code has that (and works). It's just > > that > > - memoryallocators(9) does not cover this case > > - kmem_intr_alloc(9) is kinda deprecated - quoting the man page: > > > > These routines are for the special cases. Normally, > > pool_cache(9) should be used for memory allocation from > > interrupt context. > > > > but how would I use pool_cache(9) here? > > It's not "deprecated" per se. Heck, kmem_intr_alloc() was added > *after* the pool cache API was added :-). Sounds to me like > memoryallocators(9) needs to be combed through and updated. > > Anyway, I think what the documentation is trying to convey is that > "pool_cache is better if you are allocating and freeing fixed size > objects in a hot code path". However, you're not allocating > fixed-size objects, so using pool_cache directly is not appropriate. > Using kmem_intr_alloc() is preferable to rolling your own logic here, > and gets you the optimal behavior for this use case. > > -- thorpej > The kmem_intr_alloc is a replacement for the old malloc. I think dedicated pool_cache is useful if either there is a custom constructor/destructor or a lot of allocations to save some memory due to better fitting but that savings have to make up against the pool_cache structures itself. That's why we have kmem_intr_alloc a pool_cache just to have a interrupt safe allocator for a single use case is, simply spoken, just an enormous overhead. I was/am under the impression that the longtime goal is to move all allocations out of interrupt paths and that kmem_intr_alloc is there only for transition, I have doubts this will happen. Don't get me wrong moving allocations out of interrupt paths is a good thing. Technically kmem_alloc and kmem_intr_alloc are backed by the same pools so the distinction is only API wise. (I have a patch using different pool_caches for kmem_alloc and kmem_intr_alloc it enforces that distinctions and allows some paths in kmem_allocs pool_caches to skip interrupt disabling... I have no idea/measured how much of a difference this makes) Most systems I've looked at just have the equivalent of kmem_intr_alloc. Having to block on deallocation is even worse but lukily we don't. If I haven't missed anything technically kmem_intr_free does no allocations for freeing resources, there is one allocation but if it fails we skip caching the chunk of memory and return it directly. (Even that could be tackled by having a bound number of pool_cache_groups per pool_cache per cpu) No allocation on the free path is a nice property of vmem. para -- You will continue to suffer if you have an emotional reaction to everything that is said to you. True power is sitting back and observing everything with logic. If words control you that means everyone else can control you. Breathe and allow things to pass. --- Bruce Lee
Re: Temporary memory allocation from interrupt context
> On Nov 11, 2020, at 5:38 AM, Martin Husemann wrote: > > Yes, and of course the real code has that (and works). It's just that > - memoryallocators(9) does not cover this case > - kmem_intr_alloc(9) is kinda deprecated - quoting the man page: > > These routines are for the special cases. Normally, > pool_cache(9) should be used for memory allocation from interrupt > context. > > but how would I use pool_cache(9) here? It's not "deprecated" per se. Heck, kmem_intr_alloc() was added *after* the pool cache API was added :-). Sounds to me like memoryallocators(9) needs to be combed through and updated. Anyway, I think what the documentation is trying to convey is that "pool_cache is better if you are allocating and freeing fixed size objects in a hot code path". However, you're not allocating fixed-size objects, so using pool_cache directly is not appropriate. Using kmem_intr_alloc() is preferable to rolling your own logic here, and gets you the optimal behavior for this use case. -- thorpej
Re: Temporary memory allocation from interrupt context
> On Nov 11, 2020, at 1:44 AM, Martin Husemann wrote: > > In a perfect world we would avoid the interrupt allocation all together, but > I have not found a way to rearrange things here to make this feasible. > > Is kmem_intr_alloc(9) the best way forward? While softints are backed by threads these days, kmem_intr_alloc() is the API to use in this scenario. (As an aside, kmem_alloc() is itself actually a wrapper around kmem_intr_alloc() that merely asserts that you're not in hard- or soft-interrupt context ... historically, there used to be a real distinction, because they were backed by different VM maps with different locking protocols ... these days, it's all backed by a vmem arena). However, it is worth noting that softint threads are ONLY allowed to sleep if blocking on a mutex or rwlock; sleeping for memory allocation, or a condvar or whatever is not allowed (this is a policy decision rooted in the fact that any given softint thread can only be processing one softint at a time, and we want to prevent starvation). So, because you can't sleep, you must pass the KM_NOSLEEP flag to kmem_intr_alloc() (there's already an assertion to ensure the caller has passed exactly one of KM_SLEEP *or* KM_NOSLEEP, but we should probably add an ASSERT_SLEEPABLE() in the KM_SLEEP case to catch errors like this). If the size fits into one of the kmem_cache or kmem_cache_big buckets, the allocation *will* come out of a pool_cache, and figuring out which pool cache to use is pretty quick, so you're not being penalized too badly here for not knowing the size ahead of time. -- thorpej
Re: Temporary memory allocation from interrupt context
On Wed, Nov 11, 2020 at 03:08:12PM +0100, Joerg Sonnenberger wrote: > On Wed, Nov 11, 2020 at 10:44:45AM +0100, Martin Husemann wrote: > > Consider the following pseudo-code running in softint context: > > Why do those items not have a link element inside, so that no additional > memory allocation is necesary? That would not help - I am collecting a subset of the items and don't want to keep the whole state locked for all actions on them. A single list element inside would not be enough (they do have one, that is how the whole list works). I could create a reference struct (tuple of pointer and tailq entry) for each one collected and put that in a tailq, and then use a pool_cache(9) for the referencers - which would make the whole thing similariy akward as the kmem_intr_* variant. Martin
Re: Temporary memory allocation from interrupt context
On Wed, Nov 11, 2020 at 10:44:45AM +0100, Martin Husemann wrote: > Consider the following pseudo-code running in softint context: Why do those items not have a link element inside, so that no additional memory allocation is necesary? Joerg
Re: Temporary memory allocation from interrupt context
Martin Husemann writes: > On Wed, Nov 11, 2020 at 08:26:45AM -0500, Greg Troxel wrote: >> >LOCK(st); >> >size_t n, max_n = st->num_items; >> >some_state_item **tmp_list = >> >kmem_intr_alloc(max_n * sizeof(*tmp_list)); >> >> kmem_intr_alloc takes a flag, and it seems that you need to pass >> KM_NOSLEEP, as blocking for memory in softint context is highly unlikely >> to be the right thing. > > Yes, and of course the real code has that (and works). It's just that > - memoryallocators(9) does not cover this case > - kmem_intr_alloc(9) is kinda deprecated - quoting the man page: > > These routines are for the special cases. Normally, > pool_cache(9) should be used for memory allocation from interrupt > context. > >but how would I use pool_cache(9) here? Not deprecated, but for "special cases". I think needing a possibly-big variable-size chunk of memory at interrupt time is special. You would use pool_cache by being able to use a fixed-sized object. But it seems that's not how the situation is. I think memoryallocators(9) could use some spiffing up; it (on 9) says kmem(9) cannot be used from interrupt context. The central hard problem is orthogonal, though: if you don't pre-allocate, you have to choose between waiting and copying with failure. signature.asc Description: PGP signature
Re: Temporary memory allocation from interrupt context
On Wed, Nov 11, 2020 at 08:26:45AM -0500, Greg Troxel wrote: > > LOCK(st); > > size_t n, max_n = st->num_items; > > some_state_item **tmp_list = > > kmem_intr_alloc(max_n * sizeof(*tmp_list)); > > kmem_intr_alloc takes a flag, and it seems that you need to pass > KM_NOSLEEP, as blocking for memory in softint context is highly unlikely > to be the right thing. Yes, and of course the real code has that (and works). It's just that - memoryallocators(9) does not cover this case - kmem_intr_alloc(9) is kinda deprecated - quoting the man page: These routines are for the special cases. Normally, pool_cache(9) should be used for memory allocation from interrupt context. but how would I use pool_cache(9) here? Martin
Re: Temporary memory allocation from interrupt context
Martin Husemann writes:
> Consider the following pseudo-code running in softint context:
>
> void
> softint_func(some_state *st, )
> {
> LOCK(st);
> size_t n, max_n = st->num_items;
> some_state_item **tmp_list =
> kmem_intr_alloc(max_n * sizeof(*tmp_list));
kmem_intr_alloc takes a flag, and it seems that you need to pass
KM_NOSLEEP, as blocking for memory in softint context is highly unlikely
to be the right thing.
The an page is silent on whether lack of both flags is an error, and if
not what the semantics are. (It seems to me it should be an error.)
With KM_NOSLEEP, it is possible that the allocation will fail. Thus
there needs to be a strategy to deal with that.
> n = 0;
> for (i : st->items) {
> if (!(i matches some predicate))
> continue;
> i->retain();
> tmp_list[n++] = i;
> }
> UNLOCK(st);
> /* do something with all elements in tmp_list */
> kmem_intr_free(tmp_list, max_n * sizeof(*tmp_list));
> }
>
> I don't want to alloca here (the list could be quite huge) and max_n could
> vary a lot, so having a "manual" pool of a few common (preallocated)
> list sizes hanging off the state does not go well either.
I think that you need to pick one of
pre-allocate the largest size and use it temporarily
be able to deal with not having memory. This leads to hard-to-debug
situations if that code is wrong, becuase usually malloc will succeed.
figure out that this softint can block indefinitely, only harming
later calls of the same family, and not leading to kernel
deadlock/etc. This leads to hard-to-debug situations if lack of
memory does lead to hangs, because usually malloc will succeed.
> In a perfect world we would avoid the interrupt allocation all together, but
> I have not found a way to rearrange things here to make this feasible.
>
> Is kmem_intr_alloc(9) the best way forward?
With all that said, note that I'm not the allocation export.
signature.asc
Description: PGP signature
