All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jesse Barnes <jbarnes@virtuousgeek.org>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] Antigcc bitfield bikeshed
Date: Thu, 25 Jun 2015 08:30:22 -0700	[thread overview]
Message-ID: <558C1E8E.5070106@virtuousgeek.org> (raw)
In-Reply-To: <20150625073327.GL25769@phenom.ffwll.local>

On 06/25/2015 12:33 AM, Daniel Vetter wrote:
>> In the specific case of bitfields it seems like it would be sufficient
>> > to mark the local variables as volatile?  Or maybe just use open coded
>> > compiler barrier() functions instead, with accompanying documentation.
>> > 
>> > Documentation/memory-barriers.txt is growing more and more interesting
>> > over the years (definitely more complicated than when I last added to it!).
> Yeah I'm honestly not too concerned about gcc making a mess in the two
> cases Chris want's to check something locklessly. It's more for
> documentation really so that when you read the code that special lockless
> access sticks out. Compiler barrier with a local variable might work, but
> the nice thing with ACCESS_ONCE&friends is that they also document exactly
> what the thing is you read locklessly.
> 
> Wrt comments: I thought the rule for comments on barriers is to make sure
> you don't forget to explain where the other side of the barrier is. Which
> very often is totally non-obvious. With lockless access we should have
> comments in headers already which locks protect which data (big emphasis
> on "should"), and the macros make it clear that lockless tricks are being
> played. So not sure what to add in comments. What do you have in mind?

Well I think they should document what ordering issue they're trying to
resolve.  What other code path depends on the specific ordering that the
ACCESS_ONCE provides in both of these cases?

> Aside: The two users in drm&i915 could all be replaced with READ_ONCE I
> think.

That would make things a little clearer, but we should still document
whether we're trying to make things work vs an interrupt handler, or
describe the reordering/folding/optimization we're trying to prevent
with the macro that would affect behavior.

Jesse
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

      reply	other threads:[~2015-06-25 15:30 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-17 12:47 [PATCH] Antigcc bitfield bikeshed Chris Wilson
2015-06-17 14:28 ` Jani Nikula
2015-06-17 15:10   ` Daniel Vetter
2015-06-22 21:19     ` Jesse Barnes
2015-06-23  6:53       ` Daniel Vetter
2015-06-24 22:56         ` Jesse Barnes
2015-06-25  7:33           ` Daniel Vetter
2015-06-25 15:30             ` Jesse Barnes [this message]

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=558C1E8E.5070106@virtuousgeek.org \
    --to=jbarnes@virtuousgeek.org \
    --cc=daniel@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.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.