From: Dave Chinner <david@fromorbit.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Elena Reshetova <elena.reshetova@intel.com>,
darrick.wong@oracle.com, linux-kernel@vger.kernel.org,
linux-xfs@vger.kernel.org, keescook@chromium.org
Subject: Re: [PATCH 0/5] xfs refcount conversions
Date: Fri, 3 Nov 2017 11:23:05 +1100 [thread overview]
Message-ID: <20171103002305.GZ5858@dastard> (raw)
In-Reply-To: <20171023134149.GD3165@worktop.lehotels.local>
[I missed this followup, other stuff]
On Mon, Oct 23, 2017 at 03:41:49PM +0200, Peter Zijlstra wrote:
> On Sat, Oct 21, 2017 at 10:21:11AM +1100, Dave Chinner wrote:
> > On Fri, Oct 20, 2017 at 02:07:53PM +0300, Elena Reshetova wrote:
> > IMO, that makes it way too hard to review sanely for code that:
> >
> > a) we already know works correctly
>
> But how do you know if you have unknown ordering requirements?
Because back when it was converted to atomic-based object reference
counts, I went through all the memory-barriers.txt stuff to make
sure it was OK. That was years ago, and I've forgotten it all and
the life-cycle constaints that lead us to use atomics in this
manner.
Now, I've got to go determine what the difference between atomic and
refcounts are and work them out myself because nobody has documented
it. And I have to go look at all the commit logs to work out in that
has any effect on the objects using the atomics, because that's no
longer in my head. There probably isn't an issue here, but such
changes are not done without review, and that's what is needed to
review the change.
That's the problem here - I have to work out what the differences in
ordering constraints between refcounts and atomics are myself
because it's not actually documented anywhere for reviewiers to
understand. That's a significant burden to put on a reviewer for
what is supposed to be a "no-op" change.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
next prev parent reply other threads:[~2017-11-03 0:23 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-20 11:07 [PATCH 0/5] xfs refcount conversions Elena Reshetova
2017-10-20 11:07 ` [PATCH 1/5] fs, xfs: convert xfs_bui_log_item.bui_refcount from atomic_t to refcount_t Elena Reshetova
2017-10-20 11:07 ` [PATCH 2/5] fs, xfs: convert xfs_efi_log_item.efi_refcount " Elena Reshetova
2017-10-20 11:07 ` [PATCH 3/5] fs, xfs: convert xlog_ticket.t_ref " Elena Reshetova
2017-10-20 11:07 ` [PATCH 4/5] fs, xfs: convert xfs_cui_log_item.cui_refcount " Elena Reshetova
2017-10-20 11:07 ` [PATCH 5/5] fs, xfs: convert xfs_rui_log_item.rui_refcount " Elena Reshetova
2017-10-20 23:21 ` [PATCH 0/5] xfs refcount conversions Dave Chinner
2017-10-23 10:29 ` Reshetova, Elena
2017-10-23 13:41 ` Peter Zijlstra
2017-11-03 0:23 ` Dave Chinner [this message]
2017-11-03 8:19 ` Reshetova, Elena
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=20171103002305.GZ5858@dastard \
--to=david@fromorbit.com \
--cc=darrick.wong@oracle.com \
--cc=elena.reshetova@intel.com \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
--cc=peterz@infradead.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.