Re: [PATCH] docs: rust: extend abstraction and binding documentation

[Benno Lossin <benno.lossin@proton.me>]

On 2/7/24 06:32, Behme Dirk (CM/ESO2) wrote:
> On 06.02.2024 20:35, Miguel Ojeda wrote:
>> On Tue, Feb 6, 2024 at 7:54 AM Dirk Behme <dirk.behme@de.bosch.com> wrote:
>>>
>>> CC: Miguel Ojeda <ojeda@kernel.org>
>>
>> Nit: it is typically spelled Cc (i.e. camel case).
>>
>>> +.. code-block::
>>> +
>>> +                                                       rust/bindings/
>>> +                                                       (rust/helpers.c)
>>> +
>>> +                                                          include/ -----+ <-+
>>> +                                                                        |   |
>>> +         drivers/              rust/kernel/              +----------+ <-+   |
>>> +           fs/                                           | bindgen  |       |
>>> +          .../            +-------------------+          +----------+ --+   |
>>> +                          |    Abstractions   |                         |   |
>>> +       +---------+        | +------+ +------+ |          +----------+   |   |
>>> +       | my_foo  | -----> | | foo  | | bar  | | -------> | Bindings | <-+   |
>>> +       | driver  |  Safe  | | sub- | | sub- | |  Unsafe  |          |       |
>>> +       +---------+        | |system| |system| |          | bindings | <-----+
>>> +            |             | +------+ +------+ |          |  crate   |       |
>>> +            |             |   kernel crate    |          +----------+       |
>>> +            |             +-------------------+                             |
>>> +            |                                                               |
>>> +            +------------------# FORBIDDEN #--------------------------------+
>>
>> Thanks a lot for the effort of making that slide into ASCII :)
>>
>> I could try to see if I can export the diagram into SVG and include it.
>>
>>> +The main idea is to encapsulate all ``unsafe()`` handling in the abstractions reviewed
>>> +and documented carefully. These are considered to be sound then. With this model
>>> +it is assumed that users of the abstractions ("my_foo driver") can't do anything
>>> +unsound if
>>> +
>>> +#. the abstractions are sound
>>> +#. they don't use ``unsafe()``
>>
>> Like Valentin, I feel the wording needs some improvement here. Part of
>> the confusion may be the `unsafe()` there -- it is unclear what it
>> means (especially given the parentheses).
>>
>> It is true we want to encapsulate C APIs and we forbid calling
>> directly `bindings` outside subsystems / exceptional cases; and
>> ideally provide safe abstractions as much as possible. However, we are
>> not the ones "considering" the abstractions sound (or at least that
>> way of putting it may confuse readers). We do our best to have sound
>> abstractions, but it is not something we "decide". So if e.g. someone
>> passes a raw pointer and just dereferences it in a safe function, that
>> is most likely unsound and it should be fixed.
>>
>> That is, in turn, what allows callers to avoid breaking things. In
>> other words, we don't "assume that users cannot do anything unsound"
>> (introduce UB?) -- quite the opposite: what we want (as much as
>> possible) is that they cannot introduce UB even if they tried.
>>
>> And that applies even if users use `unsafe`, as long as they use it
>> correctly, i.e. as long as they don't break the preconditions of what
>> they use. It is just that the best case is that they do not even need
>> to use `unsafe` calls (this is why we want that APIs are as safe as
>> possible, because then we minimize the chances that users get it wrong
>> and introduce UB).
>>
>>> +Abstractions
>>> +~~~~~~~~~~~~
>>> +
>>> +Abstractions are the glue layer between the bindings and the in-kernel users. E.g.
>>
>> Perhaps we should avoid "glue layer", since that sometimes implies
>> that it is a "thin" layer / "uninteresting" layer. However, the
>> abstractions are the actual interesting bits (and can be quite
>> complex, precisely to achieve the relaxation of safety preconditions,
>> i.e. making APIs "as safe as possible", that you mention below).
>>
>> But it is quite subjective.
>>
>>> +The webinar `Rust For Linux: Writing Safe Abstractions & Drivers`
>>> +
>>> +       https://www.linuxfoundation.org/webinars/rust-for-linux-writing-abstractions-and-drivers
>>> +
>>> +has an introduction to this topic.
>>
>> I appreciate this, though some of the things in there are outdated, so
>> I am not sure about including this nowadays. Perhaps we should record
>> new ones instead :) Or, even better, I could "write" the talk in
>> written form here, which can be maintained and be kept up to date.
> 
> 
> Yes, that sounds really great! :)
> 
> 
>> Relatedly, Benno is working on some extra documentation about how to
>> write safety preconditions that we will probably add around here when
>> it is ready.
> 
> Benno, is it worth to wait for that? I.e. drop this patch and wait for
> your version?

My stuff focuses more on how to to write safety documentation and what
our motivations for certain conventions are. This gives a higher level
overview and useful on its own, so you can keep it.
I agree with Miguel that some of the wording should be improved. Maybe
he can help you with that.

-- 
Cheers,
Benno