All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boqun Feng <boqun.feng@gmail.com>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: "Gary Guo" <gary@garyguo.net>,
	"Peter Zijlstra" <peterz@infradead.org>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Will Deacon" <will@kernel.org>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Wedson Almeida Filho" <wedsonaf@gmail.com>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Vincenzo Palazzo" <vincenzopalazzodev@gmail.com>
Subject: Re: [RFC 2/5] rust: sync: Arc: Introduces ArcInner::count()
Date: Thu, 2 Feb 2023 08:51:51 -0800	[thread overview]
Message-ID: <Y9vqJ1h2nkaFRpOY@boqun-archlinux> (raw)
In-Reply-To: <Y9viM2POUsSnbKUh@kroah.com>

On Thu, Feb 02, 2023 at 05:17:55PM +0100, Greg KH wrote:
[...]
> > > > But there are correct ways to use a refcount, e.g. if you own
> > > > `Arc` and `.count()` returns 1, then you know that you are the
> > > > exclusive owner of the `Arc` and nobody else is going to touch it.
> > > 
> > > But you should never know this, as it is not relevant.
> > > 
> > > So no, please don't allow printing out of a reference count, that will
> > > only cause problems and allow people to think it is safe to do so.
> > > 
> > 
> > People already do it, even in *security* code,
> > 
> > security/keys/keyring.c:
> > 
> > 	int key_link(struct key *keyring, struct key *key)
> > 	{
> > 		...
> > 		kenter("{%d,%d}", keyring->serial, refcount_read(&keyring->usage));
> > 		...
> > 	}
> > 
> > Should we fix that?
> 
> Yes.  But really, that's debugging code, it probably should all be
> removed now.
> 

Well, there are also printings in proc_keys_show() and
proc_key_users_show() in security/keys/proc.c, and I think it's hard to
remove these since they are userspace related.

Anyway I realise I could have done a better job explaining what I'm
doing here:

I want to read refcount in a later patch, which make Arc<T> implement
Debug trait/interface, and that allows user to print Arc<T> for debug
purposes, e.g.

	pr_info!("{:#x?}", a); // a is an "Arc<T">

that's the only usage of the reading from refcount. I could open code an
FFI call in that implementation, but I thought maybe I could add a
helper function, hence the "count" function. And since "count" is a
private function, so no one can use it outside this
rust/kernel/sync/arc.rs file, therefore mis-usage by outsiders are
impossible.

Maybe I made things confusing because I just learned the language and
wanted to try out a few things which made things complicated (for
review), hope the above explanation undo some of the confusion I
created.

As I said, I'm open to remove the printing of the refcount, and if you
and Peter think maybe it's OK to do that after the explanation above,
I will improve the patch to make things clear ;-)

Regards,
Boqun

> thanks,
> 
> greg k-h

  reply	other threads:[~2023-02-02 16:52 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-01 23:22 [RFC 0/5] rust: sync: Arc: Implement Debug and Display Boqun Feng
2023-02-01 23:22 ` [RFC 1/5] rust: sync: impl Display for {Unique,}Arc Boqun Feng
2023-02-02 14:15   ` Gary Guo
2023-02-02 16:50   ` Björn Roy Baron
2023-02-04 10:20   ` Finn Behrens
2023-02-04 18:47   ` Vincenzo Palazzo
2023-02-01 23:22 ` [RFC 2/5] rust: sync: Arc: Introduces ArcInner::count() Boqun Feng
2023-02-02  9:14   ` Peter Zijlstra
2023-02-02 13:46     ` Boqun Feng
2023-02-02 14:21     ` Gary Guo
2023-02-02 15:41       ` Greg KH
2023-02-02 16:10         ` Boqun Feng
2023-02-02 16:17           ` Greg KH
2023-02-02 16:51             ` Boqun Feng [this message]
2023-02-02 21:47               ` Miguel Ojeda
2023-02-03  5:22                 ` Greg KH
2023-02-03  7:25                   ` Boqun Feng
2023-02-03  7:38                     ` Greg KH
2023-02-03  7:43                       ` Boqun Feng
2023-02-03  8:01                       ` Boqun Feng
2023-02-03 19:17                       ` Josh Stone
2023-02-03 19:22                         ` Wedson Almeida Filho
2023-02-02 14:22   ` Gary Guo
2023-02-04 18:48   ` Vincenzo Palazzo
2023-02-01 23:22 ` [RFC 3/5] rust: sync: Arc: Introduces Arc::get_inner() helper Boqun Feng
2023-02-02 14:24   ` Gary Guo
2023-02-02 16:53   ` Björn Roy Baron
2023-02-04 18:51   ` Vincenzo Palazzo
2023-02-01 23:22 ` [RFC 4/5] rust: sync: impl Debug for {Unique,}Arc Boqun Feng
2023-02-02 14:28   ` Gary Guo
2023-02-03 19:46     ` Boqun Feng
2023-02-04 18:56   ` Vincenzo Palazzo
2023-02-01 23:22 ` [RFC 5/5] sample: rust: print: Add sampe code for Arc printing Boqun Feng
2023-02-02 16:56   ` Björn Roy Baron
2023-02-04 10:22   ` Finn Behrens
2023-02-04 19:05   ` Vincenzo Palazzo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y9vqJ1h2nkaFRpOY@boqun-archlinux \
    --to=boqun.feng@gmail.com \
    --cc=alex.gaynor@gmail.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=ojeda@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=vincenzopalazzodev@gmail.com \
    --cc=wedsonaf@gmail.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.