Add a type alias for file offsets, i.e. bindings::loff_t. Trying to
avoid using raw bindings types, this seems to be the better alternative
compared to just using i64.

Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Jan Kara <jack@suse.cz>
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
Al, Christian: If you are okay with the patch, kindly provide an ACK, so I can
take it through the driver-core tree.
---
 rust/kernel/fs/file.rs | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/rust/kernel/fs/file.rs b/rust/kernel/fs/file.rs
index cf06e73a6da0..021a6800b46c 100644
--- a/rust/kernel/fs/file.rs
+++ b/rust/kernel/fs/file.rs
@@ -17,6 +17,11 @@
 };
 use core::ptr;
 
+/// Primitive type representing the offset within a [`File`].
+///
+/// Type alias for `bindings::loff_t`.
+pub type Offset = bindings::loff_t;
+
 /// Flags associated with a [`File`].
 pub mod flags {
     /// File is opened in append mode.
-- 
2.51.0

On Tue, Oct 21, 2025 at 12:26:13AM +0200, Danilo Krummrich wrote:
> Add a type alias for file offsets, i.e. bindings::loff_t. Trying to
> avoid using raw bindings types, this seems to be the better alternative
> compared to just using i64.
> 
> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: Jan Kara <jack@suse.cz>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
> Al, Christian: If you are okay with the patch, kindly provide an ACK, so I can
> take it through the driver-core tree.
> ---

Reviewed-by: Christian Brauner <brauner@kernel.org>

>  rust/kernel/fs/file.rs | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/rust/kernel/fs/file.rs b/rust/kernel/fs/file.rs
> index cf06e73a6da0..021a6800b46c 100644
> --- a/rust/kernel/fs/file.rs
> +++ b/rust/kernel/fs/file.rs
> @@ -17,6 +17,11 @@
>  };
>  use core::ptr;
>  
> +/// Primitive type representing the offset within a [`File`].
> +///
> +/// Type alias for `bindings::loff_t`.
> +pub type Offset = bindings::loff_t;
> +
>  /// Flags associated with a [`File`].
>  pub mod flags {
>      /// File is opened in append mode.
> -- 
> 2.51.0
>

On Wed Oct 29, 2025 at 1:49 PM CET, Christian Brauner wrote:
> On Tue, Oct 21, 2025 at 12:26:13AM +0200, Danilo Krummrich wrote:
>> Add a type alias for file offsets, i.e. bindings::loff_t. Trying to
>> avoid using raw bindings types, this seems to be the better alternative
>> compared to just using i64.
>> 
>> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
>> Cc: Christian Brauner <brauner@kernel.org>
>> Cc: Jan Kara <jack@suse.cz>
>> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
>> ---
>> Al, Christian: If you are okay with the patch, kindly provide an ACK, so I can
>> take it through the driver-core tree.
>> ---
>
> Reviewed-by: Christian Brauner <brauner@kernel.org>

Thanks Christian!

Based on Miguel's suggestion [1] v3 of the series introduces a new type for
file::Offset rather than just a type alias.

Therefore, v3 [2] is significantly different, hence I will wait for you to reply
to v3 before picking up the entire series.

Thanks,
Danilo

[1] https://github.com/Rust-for-Linux/linux/issues/1198
[2] https://lore.kernel.org/lkml/20251022143158.64475-2-dakr@kernel.org/

On Tue, Oct 21, 2025 at 12:27 AM Danilo Krummrich <dakr@kernel.org> wrote:
>
> Add a type alias for file offsets, i.e. bindings::loff_t. Trying to
> avoid using raw bindings types, this seems to be the better alternative
> compared to just using i64.

Would a newtype be too painful?

Note: I didn't actually check if it is a sensible idea, but when I see
an alias I tend to ask myself that so it would be nice to know the
pros/cons (we could ideally mention why in the commit message in cases
like this).

Thanks!

Cheers,
Miguel

On Tue Oct 21, 2025 at 5:08 PM CEST, Miguel Ojeda wrote:
> On Tue, Oct 21, 2025 at 12:27 AM Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> Add a type alias for file offsets, i.e. bindings::loff_t. Trying to
>> avoid using raw bindings types, this seems to be the better alternative
>> compared to just using i64.
>
> Would a newtype be too painful?
>
> Note: I didn't actually check if it is a sensible idea, but when I see
> an alias I tend to ask myself that so it would be nice to know the
> pros/cons (we could ideally mention why in the commit message in cases
> like this).

Yeah, I don't think there's any value making this a new type in this case. There
are no type invariants, useful methods, etc.

In fact, not even the type alias is strictly needed, as i64 would be sufficient
as well.

The main motivation for the type alias is that I think i64 is not super
intuitive for an offset value (people would rather expect usize or isize) and
it's nice to not have bindings::loff_t exposed to driver APIs.

On Tue, Oct 21, 2025 at 5:26 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> Yeah, I don't think there's any value making this a new type in this case. There
> are no type invariants, useful methods, etc.
>
> In fact, not even the type alias is strictly needed, as i64 would be sufficient
> as well.

Even if there are no type invariants nor methods, newtypes are still
useful to prevent bad operations/assignments/...

In general, it would be ideal to have more newtypes (and of course
avoid primitives for everything), but they come with source code
overhead, so I was wondering here, because it is usually one chance we
have to try to go with something stronger vs. the usual C way.

Cheers,
Miguel

On Tue Oct 21, 2025 at 6:00 PM CEST, Miguel Ojeda wrote:
> On Tue, Oct 21, 2025 at 5:26 PM Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> Yeah, I don't think there's any value making this a new type in this case. There
>> are no type invariants, useful methods, etc.
>>
>> In fact, not even the type alias is strictly needed, as i64 would be sufficient
>> as well.
>
> Even if there are no type invariants nor methods, newtypes are still
> useful to prevent bad operations/assignments/...
>
> In general, it would be ideal to have more newtypes (and of course
> avoid primitives for everything), but they come with source code
> overhead, so I was wondering here, because it is usually one chance we
> have to try to go with something stronger vs. the usual C way.

We need arithmetic operations (e.g. checked_add()) and type conversions (e.g.
from/into usize). That'd be quite some code to write that just forwards to the
inner primitive.

So, I think as by now a new type makes sense if there is some reasonable
additional semantics to capture. Of course, if we'd have modular derive macros I
think it is reasonable and makes sense to also do it in cases like this one.

Maybe the action to take from this is to add this to the list of (good first)
issues (not sure this is actually a _good first_ issue though :).

On Tue, Oct 21, 2025 at 6:25 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> We need arithmetic operations (e.g. checked_add()) and type conversions (e.g.
> from/into usize). That'd be quite some code to write that just forwards to the
> inner primitive.

If it is just e.g. add/sub and a few from/into, then it sounds ideal
(i.e. I am worried if it wants to be used as essentially a primitive,
in which case I agree it would need to be automated to some degree).

> So, I think as by now a new type makes sense if there is some reasonable
> additional semantics to capture.

To me, part of that is restricting what can be done with the type to
prevent mistakes.

i.e. a type alias is essentially the wild west, and this kind of
type/concept is common enough that it sounds a good idea to pay the
price to provide a proper type for it.

> Maybe the action to take from this is to add this to the list of (good first)
> issues (not sure this is actually a _good first_ issue though :).

So I don't want to delay real work, and as long as we do it early
enough that we don't have many users, that sounds like a good idea to
me -- done:

    https://github.com/Rust-for-Linux/linux/issues/1198

Also took the chance to ask for a couple examples/tests.

I hope that helps.

Cheers,
Miguel

On Tue Oct 21, 2025 at 6:47 PM CEST, Miguel Ojeda wrote:
> If it is just e.g. add/sub and a few from/into, then it sounds ideal
> (i.e. I am worried if it wants to be used as essentially a primitive,
> in which case I agree it would need to be automated to some degree).

That's what I need for this patch series, I think the operations applicable for
this type are quite some more; not that far from a primitive.

> To me, part of that is restricting what can be done with the type to
> prevent mistakes.
>
> i.e. a type alias is essentially the wild west, and this kind of
> type/concept is common enough that it sounds a good idea to pay the
> price to provide a proper type for it.

The reason why I said for this case I think it would be good to have derive
macros first is that there's a lot of things that are valid to do with an offset
value.

Of course, there's also a lot of things that don't make a lot of sense that we
could prevent (e.g. reverse_bits(), ilogX(), etc.).

I think in this case there not a lot to gain from preventing this, considering
that we don't have derive macros yet.

However, I understand where you're coming from. Even though there's not a huge
gain here, it would be good to set an example -- especially if it's something as
cental as a file offset type.

If this is what you have in mind, let me go ahead and do it right away (at least
for the things needed by this patch series), because that's a very good reason I
think.

> So I don't want to delay real work, and as long as we do it early
> enough that we don't have many users, that sounds like a good idea to
> me -- done:
>
>     https://github.com/Rust-for-Linux/linux/issues/1198
>
> Also took the chance to ask for a couple examples/tests.

Thanks a lot for creating the issue!

(I mainly also thought of adding one for a derive macro.)

On Tue, Oct 21, 2025 at 7:34 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> However, I understand where you're coming from. Even though there's not a huge
> gain here, it would be good to set an example -- especially if it's something as
> cental as a file offset type.
>
> If this is what you have in mind, let me go ahead and do it right away (at least
> for the things needed by this patch series), because that's a very good reason I
> think.

Yeah, exactly, it should be fairly straightforward for at least the ones here.

Up to you, i.e. we can already start (the code you showed me offline
looks good) or wait for someone to send the change (or since you
started, they can still consider more operations and expand the type
etc.).

I will send a small coding guidelines note on type aliases -- I can
reference this example when added, since it is a good example I think.
:)

Thanks!

Cheers,
Miguel

On Tue, Oct 21, 2025 at 12:26:13AM +0200, Danilo Krummrich wrote:
> Add a type alias for file offsets, i.e. bindings::loff_t. Trying to
> avoid using raw bindings types, this seems to be the better alternative
> compared to just using i64.
> 
> Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: Jan Kara <jack@suse.cz>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Please consider also changing the instances of i64 in iov.rs.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>