Message-ID: <ZnLyi5mC8JtPMhun@Boquns-Mac-mini.home>
Date: Wed, 19 Jun 2024 08:00:27 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: Benno Lossin <benno.lossin@...ton.me>
Cc: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>,
	Gary Guo <gary@...yguo.net>, rust-for-linux@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org,
	llvm@...ts.linux.dev, Miguel Ojeda <ojeda@...nel.org>,
	Alex Gaynor <alex.gaynor@...il.com>,
	Wedson Almeida Filho <wedsonaf@...il.com>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Andreas Hindborg <a.hindborg@...sung.com>,
	Alice Ryhl <aliceryhl@...gle.com>,
	Alan Stern <stern@...land.harvard.edu>,
	Andrea Parri <parri.andrea@...il.com>,	Will Deacon <will@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Nicholas Piggin <npiggin@...il.com>,	David Howells <dhowells@...hat.com>,
	Jade Alglave <j.alglave@....ac.uk>,	Luc Maranget <luc.maranget@...ia.fr>,
	"Paul E. McKenney" <paulmck@...nel.org>,
	Akira Yokosawa <akiyks@...il.com>,	Daniel Lustig <dlustig@...dia.com>,
	Joel Fernandes <joel@...lfernandes.org>,
	Nathan Chancellor <nathan@...nel.org>,
	Nick Desaulniers <ndesaulniers@...gle.com>,	kent.overstreet@...il.com,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, elver@...gle.com,
	Mark Rutland <mark.rutland@....com>,
	Thomas Gleixner <tglx@...utronix.de>,	Ingo Molnar <mingo@...hat.com>,
 Borislav Petkov <bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
	"H. Peter Anvin" <hpa@...or.com>,
	Catalin Marinas <catalin.marinas@....com>,	torvalds@...ux-foundation.org,
 linux-arm-kernel@...ts.infradead.org,	linux-fsdevel@...r.kernel.org,
 Trevor Gross <tmgross@...ch.edu>,	dakr@...hat.com
Subject: Re: [RFC 2/2] rust: sync: Add atomic support

On Wed, Jun 19, 2024 at 09:09:46AM +0000, Benno Lossin wrote:
[...]
> >>> 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?
> 

Not really ;-) Committing to a generic interface without a vision/design
is going to be a maintenace nightmare. I will need to deal the soundness
issues and different ideas about what can be put in <..>. So I'd suggest
we take great caution before we make our decision, and I may even ask
for a formal proof of the soundness of a generic interface for key
components such as atomics. Because last time I check, safety is what
Rust brings on the table, right?

Someone may say that non generic atomics is one of the biggest mistakes
that Rust ever made. But I disagree. In fact when I saw this, my first
impression was "these guys know their stuff": there are only a few types
and operations that make sense, so putting them in a generic interface
would be over-engineer (at least at the beginning). It's better correct
than convenient/popular/aesthetical.

So would you and Gary be interested at working on such a design and
proof? Maybe someone else Cced would also be interested. Eventually we
will need the tool of soundness proof for other types as well, that's
going to be very vaulable. And having that would be the real "less work
for me" ;-)

Right now, I still plan to do in the current way (i.e. non-generic
atomcis for i32, i64, isize) because there are already users [1], 
the correctness is easy to confirm and we won't confuse users with
half-baked generic design. But should we have a sound Atomic<T> design,
I'm happy to adopt it ;-)

[...]
> >> 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.
> 
[...]
> > 
> > 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.
> 

Not going to reply the above right now, because I'm leaning more
torwards switching when we have a sound Atomic<T> design, but I want you
to know I appreciate your input and explanation.

> >> argument? Because I don't see anything else.
> >>
> >>>> - 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.
> >>>
> >>>> - 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.
> 

Ah right. And even without generics we could use some declarative
macros, and I'm happy to take a look at this and provide the alternative
approach so we can choose a better one.

[1]: https://lore.kernel.org/rust-for-linux/20240611114551.228679-2-nmi@metaspace.dk/

Regards,
Boqun

Powered by blists - more mailing lists