2024-03-14 19:03:13

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [RFC PATCH 1/5] rust: block: introduce `kernel::block::mq` module

On Thu, Mar 14, 2024 at 9:58 AM Andreas Hindborg <[email protected]> wrote:
>
> Yes, good point. Another option suggested by Miguel is that
> `__blk_mq_free_request` need not be exported at all. We can make it
> non-static and then call it from
> `rust_helper_blk_mq_free_request_internal()`. Then only the latter will
> be in the kernel image symbol table, which might be better in terms of
> not exposing `__blk_mq_free_request()` directly.

The helper is not needed, i.e. what I meant was to make it non-static
and add it to `include/linux/blk-mq.h`.

Cheers,
Miguel


2024-03-14 21:51:18

by Andreas Hindborg

[permalink] [raw]
Subject: Re: [RFC PATCH 1/5] rust: block: introduce `kernel::block::mq` module

Miguel Ojeda <[email protected]> writes:

> On Thu, Mar 14, 2024 at 9:58 AM Andreas Hindborg <[email protected]> wrote:
>>
>> Yes, good point. Another option suggested by Miguel is that
>> `__blk_mq_free_request` need not be exported at all. We can make it
>> non-static and then call it from
>> `rust_helper_blk_mq_free_request_internal()`. Then only the latter will
>> be in the kernel image symbol table, which might be better in terms of
>> not exposing `__blk_mq_free_request()` directly.
>
> The helper is not needed, i.e. what I meant was to make it non-static
> and add it to `include/linux/blk-mq.h`.

The way the current code compiles, <kernel::block::mq::Request as
kernel::types::AlwaysRefCounted>::dec_ref` is inlined into the `rnull`
module. A relocation for `rust_helper_blk_mq_free_request_internal`
appears in `rnull_mod.ko`. I didn't test it yet, but if
`__blk_mq_free_request` (or the helper) is not exported, I don't think
this would be possible?

BR Andreas

2024-03-15 00:51:15

by Andreas Hindborg

[permalink] [raw]
Subject: Re: [RFC PATCH 1/5] rust: block: introduce `kernel::block::mq` module

Andreas Hindborg <[email protected]> writes:

> Miguel Ojeda <[email protected]> writes:
>
>> On Thu, Mar 14, 2024 at 9:58 AM Andreas Hindborg <[email protected]> wrote:
>>>
>>> Yes, good point. Another option suggested by Miguel is that
>>> `__blk_mq_free_request` need not be exported at all. We can make it
>>> non-static and then call it from
>>> `rust_helper_blk_mq_free_request_internal()`. Then only the latter will
>>> be in the kernel image symbol table, which might be better in terms of
>>> not exposing `__blk_mq_free_request()` directly.
>>
>> The helper is not needed, i.e. what I meant was to make it non-static
>> and add it to `include/linux/blk-mq.h`.
>
> The way the current code compiles, <kernel::block::mq::Request as
> kernel::types::AlwaysRefCounted>::dec_ref` is inlined into the `rnull`
> module. A relocation for `rust_helper_blk_mq_free_request_internal`
> appears in `rnull_mod.ko`. I didn't test it yet, but if
> `__blk_mq_free_request` (or the helper) is not exported, I don't think
> this would be possible?
>

I just tested what you suggested Miguel, and I get a link error for
`__blk_mq_free_request` being undefined when the module is linked. This
is even though the code that calls this function lives in the `kernel`
crate, because it is inlined.

BR Andreas