On Tue, Jun 16, 2020 at 2:12 PM David Sterba <dsterba@xxxxxxx> wrote:
>
> On Mon, Jun 15, 2020 at 10:38:44AM +0100, fdmanana@xxxxxxxxxx wrote:
> > From: Filipe Manana <fdmanana@xxxxxxxx>
> >
> > This brings back an optimization that commit e678934cbe5f02 ("btrfs:
> > Remove unnecessary check from join_running_log_trans") removed, but in
> > a different form. So it's almost equivalent to a revert.
>
> I very much prefer the bit to be the synchronization mechanism, the
> logic is easy to follow instead of the cryptic barrier.
>
> The original patch came with numbers to support the 'not needed and no
> perf impact
> (https://lore.kernel.org/linux-btrfs/20190523115126.10532-1-nborisov@xxxxxxxx/)
> but it probably wasn't triggering the right load.
The patch was tested without the other patch that fixes bugs in the
"inode was logged before" check, so no regression should happen in
that case.
As I mentioned in the change log, it was fine from a performance point
of view before the bug fix, after which the hot code path can be hit
again.
And 5 processes only will probably not be enough to detect it in many
environments.
>
> [...]
> > The test robots from intel reported a -30.7% performance regression for
> > a REAIM test after commit e678934cbe5f02 ("btrfs: Remove unnecessary check
> > from join_running_log_trans").
>
> Thanks for fixing the perf regression and points for the test robot too.
