On Thu, May 4, 2023 at 1:09 AM Michal Hocko [off-list ref] wrote:

On Wed 03-05-23 08:24:19, Suren Baghdasaryan wrote:

quoted

On Wed, May 3, 2023 at 12:39 AM Michal Hocko [off-list ref] wrote:

quoted

On Mon 01-05-23 09:54:45, Suren Baghdasaryan wrote:
[...]

quoted

+struct codetag_ctx *alloc_tag_create_ctx(struct alloc_tag *tag, size_t size)
+{
+     struct alloc_call_ctx *ac_ctx;
+
+     /* TODO: use a dedicated kmem_cache */
+     ac_ctx = kmalloc(sizeof(struct alloc_call_ctx), GFP_KERNEL);

You cannot really use GFP_KERNEL here. This is post_alloc_hook path and
that has its own gfp context.

I missed that. Would it be appropriate to use the gfp_flags parameter
of post_alloc_hook() here?

No. the original allocation could have been GFP_USER based and you do
not want these allocations to pullute other zones potentially. You want
GFP_KERNEL compatible subset of that mask.

Ack.

But even then I really detest an additional allocation from this context
for every single allocation request. There GFP_NOWAIT allocation for
steckdepot but that is at least cached and generally not allocating.
This will allocate for every single allocation.

A small correction here. alloc_tag_create_ctx() is used only for
allocations which we requested to capture the context. So, this last
sentence is true for allocations we specifically marked to capture the
context, not in general.

There must be a better way.

Yeah, agree, it would be good to avoid allocations in this path. Any
specific ideas on how to improve this? Pooling/caching perhaps? I
think kmem_cache does some of that already but maybe something else?
Thanks,
Suren.

--
Michal Hocko
SUSE Labs