On 16.06.24 18:30, Boqun Feng wrote:

On Sun, Jun 16, 2024 at 03:55:12PM +0000, Benno Lossin wrote:
[...]

quoted

quoted

quoted

I don't think that the idea was to "do the design later". I don't even
know how you would do that, since you need the design to submit a patch.

Then I might mis-understand Gary? He said:

"Can we avoid two types and use a generic `Atomic<T>` and then implement
on `Atomic<i32>` and `Atomic<i64>` instead?"

, which means just replace `impl AtomicI32` with `impl Atomic<i32>` to
me.

This is a fair interpretation, but what prevents you from merging the
impls of functions that can be? I assumed that you would do that
automatically.

I think you missed the point, Gary's suggestion at that time sounds
like: let's impl Atomic<i32> and Atomic<i64> first, and leave rest of
the work for later, that is a "do the design later" to me.

Hmm, but wouldn't that just be less work for you?

quoted

quoted

quoted

I can't offer you a complete API description, since that would require
me writing it up myself. But I would recommend trying to get it to work
with generics. I got a few other comments:

We should work on things that are concrete, right? It's fine that the
design is not complete, and it's fine if you just recommend. But without
a somewhat concrete design (doesn't have to be complete), I cannot be
sure about whether we have the same vision of the future of generic
atomics (see my question to Gary), that's a bit hard for me to try to

Sorry, which question?

	https://lore.kernel.org/rust-for-linux/Zm7_XWe6ciy1yN-h@Boquns-Mac-mini.home/ (local)

quoted

Also to be aligned on the vision, I think we should rather talk about
the vision and not the design, since the design that we want to have now
can be different from the vision. On that note, what do you envision the
future of the atomic API?

Mine is simple, just providing AtomicI32 and AtomicI64 first, since
there are immediate users right now, and should we learn more needs from
the users, we get more idea about what a good API is, and we evolve from
there.

That is fine, but since we want to get generics in the future anyways, I
think it would be good to already implement them now to also gather
feedback on them.

quoted

quoted

work something out (plus I personally don't think it's a good idea, it's
OK to me, but not good). Anyway, I wasn't trying to refuse to do this
just based on personal reasons, but I do need to understand what you are
all proposing, because I don't have one myself.

I went back through the thread and here is what I think your argument
against generics is: people should think about size and alignment when
using atomics, which is problematic when allowing users to leave the
atomic generic.
But as I argued before, this is not an issue. Have I overlooked another

You mean you said it's a non-issue but gave me two counteract? If it's
really a non-issue, you won't need a couneraction, right? In other words
non-generic atomics do provide some value.

The second counteractions would provide exactly the same API surface as
your non-generic version, so I don't see how going non-generic provides
any value over going generic.
The first approach was intended for a future in which we are not scared
of people using generic atomics in their structs. I don't know if we are
going to be in that future, so I think we should go with the second
approach for the time being.

quoted

argument? Because I don't see anything else.

quoted

quoted

- I don't think that we should resort to a script to generate the Rust
  code since it prevents adding good documentation & examples to the
  various methods. AFAIU you want to generate the functions from
  `scripts/atomic/atomics.tbl` to keep it in sync with the C side. I
  looked at the git log of that file and it hasn't been changed
  significantly since its inception. I don't think that there is any
  benefit to generating the functions from that file.

I'll leave this to other atomic maintainers.

quoted

- most of the documented functions say "See `c_function`", I don't like
  this, can we either copy the C documentation (I imagine it not
  changing that often, or is that assumption wrong?) or write our own?

You're not wrong. AN in C side, we do have some documentation template
to generate the comments (see scripts/atomic/kerneldoc). But first the
format is for doxygen(I guess?), and second as you just bring up, the
templates are tied with the bash script.

I see a bash script similarly to how Wedson sees proc macros ;)
We should try *hard* to avoid them and only use them when there is no
other way.

I will just start with the existing mechanism and try to evolve, whether
it's a script or proc macro, I don't mind, I want to get the work done
and most relevant people can understand in the way the they prefer and
step-by-step, move it to the place I think is the best for the project.

I don't think that we need a script or a proc macro. A few declarative
macros probably suffice if we go the way of generics.

quoted

quoted

quoted

- we should try to use either const generic or normal parameters for the
  access ordering instead of putting it in the function name.

Why is it important? Keeping it in the current way brings the value that
it's not too much different than their C counterparts. Could you explain
a bit the pros and cons on suffix vs const generic approach?

Reduce code duplication, instead of 3 different variants, we can have
one. It allows people to build ergonomic APIs that allows the user to
decide which synchronization they use under the hood.

I already mentioned why I think it's good in the current way, I will
defer it to others on their inputs.

quoted

quoted

quoted

- why do we need both non-return and return variants?

Historical reason I guess. Plus maybe some architectures have a better
implementation on non-return atomics IIRC.

Could we get some more concrete arguments for why we would need these in
rust? If the reason is purely historical, then we shouldn't expose the

Sure. Look like my memory is correct, at least on ARM64 they are
different instructions (see arch/arm64/include/asm/atomic_lse.h)

non-return atomics on ARM64:

	#define ATOMIC_OP(op, asm_op)						\
	static __always_inline void						\
	__lse_atomic_##op(int i, atomic_t *v)					\
	{									\
		asm volatile(							\
		__LSE_PREAMBLE							\
		"	" #asm_op "	%w[i], %[v]\n"				\
		: [v] "+Q" (v->counter)						\
		: [i] "r" (i));							\
	}

value-return atomics on ARM64:

	#define ATOMIC_FETCH_OP(name, mb, op, asm_op, cl...)			\
	static __always_inline int						\
	__lse_atomic_fetch_##op##name(int i, atomic_t *v)			\
	{									\
		int old;							\
										\
		asm volatile(							\
		__LSE_PREAMBLE							\
		"	" #asm_op #mb "	%w[i], %w[old], %[v]"			\
		: [v] "+Q" (v->counter),					\
		  [old] "=r" (old)						\
		: [i] "r" (i)							\
		: cl);								\
										\
		return old;							\
	}

It may not be easy to see the different instructions from the pasted
code, but you can find them in the header file, also you could notice
that the number of operands is different?

This is not my expertise, so I believe you :)

quoted

non-return variant IMO. If it is because of performance, then we can
only expose them in the respective arches.

Hmm.. so we ask user to write arch-specific code like:

	pub fn increase_counter(&self) {
	    #[cfg(CONFIG_ARM64)]
	    self.counter.inc();

	    #[cfg(CONFIG_X86_64)]
	    let _ = self.counter.inc_return();
	}

are you sure it's a good idea?

No that looks horrible.

Maybe there is something that we can do with generics, but I don't know
if it is worth it. I guess we can leave that open for the time being.

---
Cheers,
Benno