Re: [PATCH] fs-writeback: drop wb->list_lock during blk_finish_plug()

From: Linus Torvalds <torvalds@linux-foundation.org>
To: Dave Chinner <david@fromorbit.com>
Cc: Chris Mason <clm@fb.com>, Jan Kara <jack@suse.cz>,
	Josef Bacik <jbacik@fb.com>, LKML <linux-kernel@vger.kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Neil Brown <neilb@suse.de>, Christoph Hellwig <hch@lst.de>,
	Tejun Heo <tj@kernel.org>
Subject: Re: [PATCH] fs-writeback: drop wb->list_lock during blk_finish_plug()
Date: Wed, 16 Sep 2015 18:12:29 -0700	[thread overview]
Message-ID: <CA+55aFw3=_asAhUR3=o0pv0vtOJpownyWJpAfgSVtJVeaX0+bQ@mail.gmail.com> (raw)
In-Reply-To: <20150917003738.GN3902@dastard>

On Wed, Sep 16, 2015 at 5:37 PM, Dave Chinner <david@fromorbit.com> wrote:
>
> TL;DR: Results look really bad - not only is the plugging
> problematic, baseline writeback performance has regressed
> significantly.

Dave, if you're testing my current -git, the other performance issue
might still be the spinlock thing.

I haven't gotten the -tip pull fox the spinlock regression yet, and
since you were the only one who noticed, I didn't consider it critical
enough to bypass the normal schedule, so -rc1 still has the crazy-bad
test-and-set loop for spinlocks.

So it's waiting in the locking fix branch (and is cc'd for stable),
but hasn't reached me yet.

If you want to test that theory out, you can make virt_spin_lock()
just unconditionally return false to disable the bad cmpxchg loop for
now.

The plugging IO pauses are interesting, though. Plugging really
*shouldn't* cause that kind of pauses, _regardless_ of what level it
happens on, so I wonder if the patch ends up just exposing some really
basic problem that just normally goes hidden.

Can you match up the IO wait times with just *where* it is waiting? Is
it waiting for that inode I_SYNC thing in inode_sleep_on_writeback()?

But it does sound like we should just revert the whole plugging for
now, if only because "it has odd effects".

                Linus