linux-kernel - Re: [RFC PATCH 1/8] rust: kernel: add basic abstractions for device-mapper

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 17 Jul 2023 11:04:44 +0800
From:   "Qingsong Chen" <changxian.cqs@...group.com>
To:     Benno Lossin <benno.lossin@...ton.me>
Cc:     <linux-kernel@...r.kernel.org>,
        "田洪亮" <tate.thl@...group.com>,
        "Miguel Ojeda" <ojeda@...nel.org>,
        "Alex Gaynor" <alex.gaynor@...il.com>,
        "Wedson Almeida Filho" <wedsonaf@...il.com>,
        "Boqun Feng" <boqun.feng@...il.com>, "Gary Guo" <gary@...yguo.net>,
        "Björn Roy Baron" <bjorn3_gh@...tonmail.com>,
        "Martin Rodriguez Reboredo" <yakoyoku@...il.com>,
        "Alice Ryhl" <aliceryhl@...gle.com>,
        "Vincenzo Palazzo" <vincenzopalazzodev@...il.com>,
        "Asahi Lina" <lina@...hilina.net>, <rust-for-linux@...r.kernel.org>
Subject: Re: [RFC PATCH 1/8] rust: kernel: add basic abstractions for
 device-mapper

On 7/14/23 11:48 PM, Benno Lossin wrote:
>> - Declare `vtable` for basic target operations.
>> - Wrap `target_type` to register/unregister target.
>> - Wrap `dm_target`/`dm_dev` to handle io request.
>> - Add a dummy `bio` wrapper.
>>
>> Signed-off-by: Qingsong Chen <changxian.cqs@...group.com>
> 
> On my system this patch series (I did not test which patch exactly) does
> not compile.
> 
> I have left some comments below, they show some patterns present in
> the other patches as well.

Thanks for the comments. Maybe `CONFIG_BLK_DEV_ZONED=y` is needed to
compile this patchset. This is because I cannot define two functions
with the same name in a trait (using the `vtable` macro), just like
the C definition:
```c
#ifdef CONFIG_BLK_DEV_ZONED
typedef int (*dm_report_zones_fn) (struct dm_target *ti,
                                     struct dm_report_zones_args *args,
                                     unsigned int nr_zones);
#else
typedef int (*dm_report_zones_fn) (struct dm_target *dummy);
#endif
```
To fix the `vtable`, I send a little patch[1] the other day. Based on
that, the compile error of this patchset should be fixed too.

[1]:
https://lore.kernel.org/rust-for-linux/20230626074242.3945398-1-changxian.cqs@antgroup.com/

>> +/// The return values of target map function, i.e., [`TargetOperations::map`].
>> +#[repr(u32)]
>> +pub enum MapState {
>> +    /// The target will handle the io by resubmitting it later.
>> +    Submitted = bindings::DM_MAPIO_SUBMITTED,
>> +
>> +    /// Simple remap complete.
>> +    Remapped = bindings::DM_MAPIO_REMAPPED,
>> +
>> +    /// The target wants to requeue the io.
>> +    Requeue = bindings::DM_MAPIO_REQUEUE,
>> +
>> +    /// The target wants to requeue the io after a delay.
>> +    DelayRequeue = bindings::DM_MAPIO_DELAY_REQUEUE,
>> +
>> +    /// The target wants to complete the io.
>> +    Kill = bindings::DM_MAPIO_KILL,
>> +}
> 
> Is it really necessary to have these correspond to the exact values?
> Could we also just have a conversion function and leave the repr to
> the compiler?

Sure, we can have a conversion function to return its value to C side
(without `as _`).

Regards,
Qingsong