public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Danilo Krummrich" <dakr@kernel.org>
To: "Linus Torvalds" <torvalds@linux-foundation.org>
Cc: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Saravana Kannan" <saravanak@kernel.org>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	<driver-core@lists.linux.dev>, <linux-kernel@vger.kernel.org>
Subject: Re: [GIT PULL] Driver core fixes for 7.1-rc1
Date: Mon, 20 Apr 2026 02:04:12 +0200	[thread overview]
Message-ID: <DHXJJISFLXVK.2KURIQTX2A763@kernel.org> (raw)
In-Reply-To: <CAHk-=wgKsB6SqFbOgPVpBLh5vz1T+h4MxTX1qDEYH=KH9=bXAA@mail.gmail.com>

On Sun Apr 19, 2026 at 10:28 PM CEST, Linus Torvalds wrote:
> Ugh. This just looks disgusting:
>
>         device_lock(dev);
>         dev_set_ready_to_probe(dev);
>         device_unlock(dev);
>
> when all it does is to just set a single bit.

It does look ugly indeed, but I think apart from that it is not that bad; the
device lock - except for the exact case where dev_ready_to_probe() is checked -
should otherwise be uncontended.

For context, in the original version of the patch it was just another C bitfield
at the end of struct device.

I noticed that this is racy as adjacent bitfield members are not all protected
by the same lock; subsequent patches from Doug (queued for 7.2) convert the C
bitfield over to this bitmap.

IOW, the atomic bitops are primarily an implementation detail to prevent
concurrent modifications of different flags from corrupting each other.

> Sadly, I think despite being disgusting, our bitop memory ordering
> models are incomplete.
>
> But I think dev_set_ready_to_probe() could/should use
> 'test_and_set_bit()', which turns the bit setting into strongly
> ordered (it only needs "release" consistency, but we don't have that).
>
> And the dev_ready_to_probe() should use "test_bit_acquire()".

To be honest, I did not consider this as we generate all accessors with a macro
and all other bits that we convert over from the struct device's C bitfields
would inherit these semantics.

Maybe that's not necessarily a bad thing though -- another option would be to
add dev_set_##accessor_name##_release() and dev_##accessor_name##_acquire().

- Danilo

  reply	other threads:[~2026-04-20  0:04 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-19 15:43 [GIT PULL] Driver core fixes for 7.1-rc1 Danilo Krummrich
2026-04-19 20:28 ` Linus Torvalds
2026-04-20  0:04   ` Danilo Krummrich [this message]
2026-04-19 21:53 ` pr-tracker-bot

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=DHXJJISFLXVK.2KURIQTX2A763@kernel.org \
    --to=dakr@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=driver-core@lists.linux.dev \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rafael@kernel.org \
    --cc=saravanak@kernel.org \
    --cc=torvalds@linux-foundation.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox