landlock-lsm / linux

Linux kernel - See Landlock issues
https://git.kernel.org/pub/scm/linux/kernel/git/mic/linux.git/
Other
33 stars 9 forks source link

Use dedicated SLABs #19

Closed l0kod closed 2 weeks ago

l0kod commented 5 months ago

Using a kmem_cache per Landlock's kernel type could improve performance, and it would also be useful to get some metrics via /proc/slabinfo.

One SLAB per:

ayush-0110 commented 3 months ago

Hi, I am Ayush, an outreachy applicant. I have worked on several patches and got them reviewed as mentioned in project guidelines. I want to work on this issue. Can you please assign it to me and help me get started on the same? @l0kod @AlisonSchofield

l0kod commented 3 months ago

Hi Ayush!

The idea is to replace kzalloc() calls with kmem_cache_zalloc() and update the related dependencies. You should find plenty of examples in the kernel code (e.g. lsm_file_cache).

However, struct landlock_ruleset would not work because it contains a flexible array member (FAM). With a separate patch series, we could leverage the upcoming kmem_buckets_alloc().

l0kod commented 3 months ago

I'm assigning this issue to you. Please regularly update it with your progress. This doesn't need to be long but just enough to make sure you are not blocked.

ayush-0110 commented 3 months ago

Thanks a lot @l0kod . Will definitely keep updating progress and seek your guidance if anywhere I feel stuck.

ayush-0110 commented 3 months ago

Hello @l0kod . I have gone through the lsm_file_cache code for reference and tried to use kmem_cache_zalloc( ) for struct landlock_object. Let me share the steps taken, kindly correct me wherever I am wrong:

  1. created a struct of type kmem_cache to create cache.
  2. created one initialization function to create cache to allocate memory from.
  3. replaced zalloc calls with kmem_cache_zalloc( ).

the problem I am facing is: I am not getting where to call the initialisation function to ensure cache is created only once. could you help me please? @l0kod @AlisonSchofield

l0kod commented 3 months ago

Hello @l0kod . I have gone through the lsm_file_cache code for reference and tried to use kmem_cache_zalloc( ) for struct landlock_object. Let me share the steps taken, kindly correct me wherever I am wrong:

  1. created a struct of type kmem_cache to create cache.
  2. created one initialization function to create cache to allocate memory from.
  3. replaced zalloc calls with kmem_cache_zalloc( ).

This looks good.

the problem I am facing is: I am not getting where to call the initialisation function to ensure cache is created only once. could you help me please? @l0kod @AlisonSchofield

You can call the initialization function from landlock_init().

ayush-0110 commented 3 months ago

Okay. Thanks @l0kod . I am sending the changes for your review. Kindly look into them and instruct me further. Thanks.

l0kod commented 3 months ago

For reference: the related patch.

ayush-0110 commented 3 months ago

Hello @l0kod. I was trying to work on the suggestion of Paul and I am facing some problem regarding replacing kzalloc with kmem_cache_zalloc calls when using KMEM macro from include/linux/slab.h because for kmem_cache_zalloc I will be needing a cache pointer, but KMEM macro doesn't return or allocate any such pointer. So is there any way to do this using macro or do i have to avoid using that macro for this case and use all methods regarding kmem as defined in the linux memory management API doc?

ayush-0110 commented 3 months ago

Hello @l0kod . I sent a V2 of the patch. Could you please look into it and suggest me if any changes have to be done? Thanks

ayush-0110 commented 3 months ago

Hello @l0kod @AlisonSchofield. I have created a complete patch aimed at solving this issue where I have used kmem to replace the kzalloc calls with kmem_cache_zalloc and also have metrics using /proc/slabinfo. Kindly look into it. Thanks Link: the related patch

ayush-0110 commented 2 months ago

Hello @l0kod @AlisonSchofield. Kindly look into the patch I created as today is the last date for submitting our final application.

l0kod commented 2 months ago

Hello @l0kod @AlisonSchofield. Kindly look into the patch I created as today is the last date for submitting our final application.

You can use the link to the latest patch as contribution. I'll review it shortly.

ayush-0110 commented 2 months ago

Ok @l0kod . Thanks. Now I wish to change the calls for struct landlock_ruleset as well and as you mentioned, i have to use kmem_buckets_alloc(). Can you guide me about how to proceed with it?

l0kod commented 2 months ago

It's too soon to work on kmem_buckets_alloc() because of the ongoing discussion. We'll see what happen.

As suggested in the mailing list, some metrics would be useful to know the impact of kmem_cache.

ayush-0110 commented 2 months ago

Hello @l0kod. In my recent patch, I included some metrics from /proc/slabinfo, as that was mentioned in the issue. Any other metrics also needed for the same??

l0kod commented 2 months ago

Hello @l0kod. In my recent patch, I included some metrics from /proc/slabinfo, as that was mentioned in the issue. Any other metrics also needed for the same??

By metric I mean to measure the performance impact of such change. This should help: #24

l0kod commented 2 weeks ago

I guess @ayush-0110 is not working on this task anymore. Let's close this task until we get the benchmark tools, see #24 and upstream comments.