writeback livelock fixes

From: Jan Kara <[email protected]>

Background writeback are easily livelockable (from a definition of their target).

This is inconvenient because it can make sync(1) stall forever waiting on its queued work to be finished.

Generally, when a flusher thread has
some work queued, someone submitted the work to achieve a goal more specific than what background writeback does. So it makes sense to give it a priority over a generic page cleaning.

Thus we interrupt background writeback if there is some other work to do. We return to the background writeback after completing all the queued work.

Signed-off-by Jan Kara <[email protected]>

Signed-off-by Wu Fengguang <[email protected]>

---
 fs/fs-writeback.c |    9 +++++++++
 1 file changed, 9 insertions(+)

fs/fs-writeback.c +2 -0

@@ -651,6 +651,15 @@ static long wb_writeback(struct bdi_writ
 			break;
 
 		/*
+		 * Background writeout and kupdate-style writeback are
+		 * easily livelockable. Stop them if there is other work

On Tue, 09 Nov 2010 07:09:19 +0800
Wu Fengguang <[email protected]> wrote:
I find the description to be somewhat incomplete...

Quoted 4 lines

From: Jan Kara <[email protected]>

Background writeback are easily livelockable (from a definition of their target).

*why* is background writeback easily livelockable? Under which circumstances does this happen and how does it come about?

Quoted 2 lines

This is inconvenient because it can make sync(1) stall forever waiting on its queued work to be finished.

Again, why? Because there are works queued from the flusher thread, but that thread is stuck in a livelocked state in <unspecified code location> so it is unable to service the other works? But the pocess which called sync() will as a last resort itself perform all the required IO, will it not? If so, how can it livelock?

Quoted 19 lines

Generally, when a flusher thread has
some work queued, someone submitted the work to achieve a goal more specific than what background writeback does. So it makes sense to give it a priority over a generic page cleaning.

Thus we interrupt background writeback if there is some other work to do. We return to the background writeback after completing all the queued work.

Signed-off-by Jan Kara <[email protected]>

Signed-off-by Wu Fengguang <[email protected]>

---
 fs/fs-writeback.c |    9 +++++++++
 1 file changed, 9 insertions(+)

fs/fs-writeback.c +0 -0

@@ -651,6 +651,15 @@ static long wb_writeback(struct bdi_writ
 			break;
 

fs/fs-writeback.c static long wb_writeback(struct bdi_writ · line 665

Quoted 13 lines

fs/fs-writeback.c +9 -0

 		/*
+		 * Background writeout and kupdate-style writeback are
+		 * easily livelockable. Stop them if there is other work
+		 * to do so that e.g. sync can proceed.
+		 */
+		if ((work->for_background || work->for_kupdate) &&
+		    !list_empty(&wb->bdi->work_list))
+			break;
+
+		/*
 		 * For background writeout, stop when we are below the
 		 * background dirty threshold
 		 */

So... what prevents higher priority works (eg, sync(1)) from livelocking or seriously retarding background or kudate writeout?

Hi,

Quoted 3 lines · Andrew Morton

On Tue, 09 Nov 2010 07:09:19 +0800
Wu Fengguang <[email protected]> wrote:
I find the description to be somewhat incomplete...

OK, so let me fill in the gaps ;)

Quoted 8 lines

Quoted 4 lines

From: Jan Kara <[email protected]>

Background writeback are easily livelockable (from a definition of their target).

*why* is background writeback easily livelockable? Under which circumstances does this happen and how does it come about?

Quoted 8 lines

Quoted 2 lines

This is inconvenient because it can make sync(1) stall forever waiting on its queued work to be finished.

Again, why? Because there are works queued from the flusher thread, but that thread is stuck in a livelocked state in <unspecified code location> so it is unable to service the other works? But the pocess which called sync() will as a last resort itself perform all the required IO, will it not? If so, how can it livelock?

New description which should address above questions:
Background writeback is easily livelockable in a loop in wb_writeback() by a process continuously re-dirtying pages (or continuously appending to a file). This is in fact intended as the target of background writeback is to write dirty pages it can find as long as we are over dirty_background_threshold.

But the above behavior gets inconvenient at times because no other work queued in the flusher thread's queue gets processed. In particular, since e.g. sync(1) relies on flusher thread to do all the IO for it,

sync(1) can hang forever waiting for flusher thread to do the work.

Generally, when a flusher thread has some work queued, someone submitted the work to achieve a goal more specific than what background writeback does. Moreover by working on the specific work, we also reduce amount of dirty pages which is exactly the target of background writeout. So it makes sense to give specific work a priority over a generic page cleaning.

Thus we interrupt background writeback if there is some other work to do. We return to the background writeback after completing all the queued work.

Is it better now?

Quoted 19 lines

Quoted 19 lines

Generally, when a flusher thread has
some work queued, someone submitted the work to achieve a goal more specific than what background writeback does. So it makes sense to give it a priority over a generic page cleaning.

Thus we interrupt background writeback if there is some other work to do. We return to the background writeback after completing all the queued work.

Signed-off-by Jan Kara <[email protected]>

Signed-off-by Wu Fengguang <[email protected]>

---
 fs/fs-writeback.c |    9 +++++++++
 1 file changed, 9 insertions(+)

fs/fs-writeback.c +0 -0

@@ -651,6 +651,15 @@ static long wb_writeback(struct bdi_writ
 			break;
 

Quoted 16 lines

fs/fs-writeback.c static long wb_writeback(struct bdi_writ · line 665

Quoted 13 lines

fs/fs-writeback.c +9 -0

 		/*
+		 * Background writeout and kupdate-style writeback are
+		 * easily livelockable. Stop them if there is other work
+		 * to do so that e.g. sync can proceed.
+		 */
+		if ((work->for_background || work->for_kupdate) &&
+		    !list_empty(&wb->bdi->work_list))
+			break;
+
+		/*
 		 * For background writeout, stop when we are below the
 		 * background dirty threshold
 		 */

So... what prevents higher priority works (eg, sync(1)) from livelocking or seriously retarding background or kudate writeout?

If other work than background or kupdate writeout livelocks, it's a bug
which should be fixed (either by setting sensible nr_to_write or by tagging like we do it for WB_SYNC_ALL writeback). Of course, higher priority work can be running when background or kupdate writeout would need to run as well. But the idea here is that the purpose of background/kupdate types of writeout is to get rid of dirty data and any type of writeout does this so working on it we also work on background/kupdate writeout only possibly less efficiently.

Quoted 6 lines · On Tue, 9 Nov 2010 23:28:27 +0100 Jan Kara <[email protected]> wrote:

New description which should address above questions:
Background writeback is easily livelockable in a loop in wb_writeback() by a process continuously re-dirtying pages (or continuously appending to a file). This is in fact intended as the target of background writeback is to write dirty pages it can find as long as we are over dirty_background_threshold.

Well. The objective of the kupdate function is utterly different.

Quoted 3 lines

But the above behavior gets inconvenient at times because no other work queued in the flusher thread's queue gets processed. In particular, since e.g. sync(1) relies on flusher thread to do all the IO for it,

That's fixable by doing the work synchronously within sync_inodes_sb(), rather than twiddling thumbs wasting a thread resource while waiting for kernel threads to do it. As an added bonus, this even makes cpu time accounting more accurate ;)

Please remind me why we decided to hand the sync_inodes_sb() work off to other threads?

Quoted 11 lines

sync(1) can hang forever waiting for flusher thread to do the work.

Generally, when a flusher thread has some work queued, someone submitted the work to achieve a goal more specific than what background writeback does. Moreover by working on the specific work, we also reduce amount of dirty pages which is exactly the target of background writeout. So it makes sense to give specific work a priority over a generic page cleaning.

Thus we interrupt background writeback if there is some other work to do. We return to the background writeback after completing all the queued work.

Quoted 10 lines

Quoted 2 lines

So... what prevents higher priority works (eg, sync(1)) from livelocking or seriously retarding background or kudate writeout?

If other work than background or kupdate writeout livelocks, it's a bug
which should be fixed (either by setting sensible nr_to_write or by tagging like we do it for WB_SYNC_ALL writeback). Of course, higher priority work can be running when background or kupdate writeout would need to run as well. But the idea here is that the purpose of background/kupdate types of writeout is to get rid of dirty data and any type of writeout does this so working on it we also work on background/kupdate writeout only possibly less efficiently.

The kupdate function is a data-integrity/quality-of-service sort of thing.

And what I'm asking is whether this change enables scenarios in which these threads can be kept so busy that the kupdate function gets interrupted so frequently that we can have dirty memory not being written back for arbitrarily long periods of time?

Quoted 20 lines · Andrew Morton

Quoted 6 lines · On Tue, 9 Nov 2010 23:28:27 +0100 Jan Kara <[email protected]> wrote:

New description which should address above questions:
Background writeback is easily livelockable in a loop in wb_writeback() by a process continuously re-dirtying pages (or continuously appending to a file). This is in fact intended as the target of background writeback is to write dirty pages it can find as long as we are over dirty_background_threshold.

Well. The objective of the kupdate function is utterly different.

Quoted 3 lines

But the above behavior gets inconvenient at times because no other work queued in the flusher thread's queue gets processed. In particular, since e.g. sync(1) relies on flusher thread to do all the IO for it,

That's fixable by doing the work synchronously within sync_inodes_sb(), rather than twiddling thumbs wasting a thread resource while waiting for kernel threads to do it. As an added bonus, this even makes cpu time accounting more accurate ;)

Quoted 2 lines

Please remind me why we decided to hand the sync_inodes_sb() work off to other threads?

Because when sync(1) does IO on it's own, it competes for the device with
the flusher thread running in parallel thus resulting in more seeks.

Quoted 11 lines

Quoted 11 lines

sync(1) can hang forever waiting for flusher thread to do the work.

Generally, when a flusher thread has some work queued, someone submitted the work to achieve a goal more specific than what background writeback does. Moreover by working on the specific work, we also reduce amount of dirty pages which is exactly the target of background writeout. So it makes sense to give specific work a priority over a generic page cleaning.

Thus we interrupt background writeback if there is some other work to do. We return to the background writeback after completing all the queued work.

...

Quoted 11 lines

Quoted 10 lines

Quoted 2 lines

So... what prevents higher priority works (eg, sync(1)) from livelocking or seriously retarding background or kudate writeout?

If other work than background or kupdate writeout livelocks, it's a bug
which should be fixed (either by setting sensible nr_to_write or by tagging like we do it for WB_SYNC_ALL writeback). Of course, higher priority work can be running when background or kupdate writeout would need to run as well. But the idea here is that the purpose of background/kupdate types of writeout is to get rid of dirty data and any type of writeout does this so working on it we also work on background/kupdate writeout only possibly less efficiently.

Quoted 7 lines

The kupdate function is a data-integrity/quality-of-service sort of thing.

And what I'm asking is whether this change enables scenarios in which these threads can be kept so busy that the kupdate function gets interrupted so frequently that we can have dirty memory not being written back for arbitrarily long periods of time?

So let me compare:
What kupdate writeback does:
queue inodes older than dirty_expire_centisecs while some inode in the queue
write MAX_WRITEBACK_PAGES from each inode queued break if nr_to_write <= 0

What any other WB_SYNC_NONE writeback (let me call it "normal WB_SYNC_NONE writeback") does:
queue all dirty inodes
while some inode in the queue
write MAX_WRITEBACK_PAGES from each inode queued
break if nr_to_write <= 0

There only one kind of WB_SYNC_ALL writeback - the one which writes everything.

So after WB_SYNC_ALL writeback (provided all livelocks are fixed ;) obviously no old data should be unwritten in memory. Normal WB_SYNC_NONE writeback differs from a kupdate one *only* in the fact that we queue all inodes instead of only the old ones.

We start writing old inodes first and

go inode by inode writing MAX_WRITEBACK_PAGES from each. Now because the queue can be longer for normal WB_SYNC_NONE writeback, it can take longer before we return to the old inodes. So if normal writeback interrupts kupdate one, it can take longer before all data of old inodes get to disk. But we always get the old data to disk - essentially at the same time at which kupdate writeback would get them to disk if dirty_expire_centisecs was 0.

Is this enough? Do you want any of this in the changelog?

Thanks for the inquiry btw. It made me cleanup my thoughts on the subject ;)

Quoted 23 lines · On Wed, 10 Nov 2010 00:56:32 +0100 Jan Kara <[email protected]> wrote:

Quoted 22 lines · Andrew Morton

Quoted 6 lines · On Tue, 9 Nov 2010 23:28:27 +0100 Jan Kara <[email protected]> wrote:

New description which should address above questions:
Background writeback is easily livelockable in a loop in wb_writeback() by a process continuously re-dirtying pages (or continuously appending to a file). This is in fact intended as the target of background writeback is to write dirty pages it can find as long as we are over dirty_background_threshold.

Well. The objective of the kupdate function is utterly different.

Quoted 3 lines

But the above behavior gets inconvenient at times because no other work queued in the flusher thread's queue gets processed. In particular, since e.g. sync(1) relies on flusher thread to do all the IO for it,

That's fixable by doing the work synchronously within sync_inodes_sb(), rather than twiddling thumbs wasting a thread resource while waiting for kernel threads to do it. As an added bonus, this even makes cpu time accounting more accurate ;)

Please remind me why we decided to hand the sync_inodes_sb() work off to other threads?

Quoted 2 lines

Because when sync(1) does IO on it's own, it competes for the device with
the flusher thread running in parallel thus resulting in more seeks.

Skeptical. Has that effect been demonstrated? Has it been shown to be a significant problem? A worse problem than livelocking the machine? ;)

If this _is_ a problem then it's also a problem for fsync/msync. But see below.

OT, but: your faith in those time-ordered inode lists is touching ;) Put a debug function in there which checks that the lists _are_ time-ordered, and call that function from every site in the kernel
which modifies the lists. I bet there are still gremlins.

Seriously, I also doubt the value of doing sync() in the flusher thread. sync() is by definition inefficient. In the block layer, it's served with less emphasis on throughput. In the VFS layer, it may sleep in inode_wait_for_writeback() and filemap_fdatawait(). In various FS, pages won't be skipped at the cost of more lock waiting.

So when a flusher thread is serving sync(), it has difficulties saturating the storage device.

btw, it seems current sync() does not take advantage of the flusher threads to sync multiple disks in parallel.

And I guess (concurrent) sync/fsync/msync calls will be rare, especially for really performance demanding workloads (which will optimize sync away in the first place).

Quoted 21 lines · On Wed, 10 Nov 2010 00:56:32 +0100 Jan Kara <[email protected]> wrote:

Quoted 20 lines · Andrew Morton

Quoted 6 lines · On Tue, 9 Nov 2010 23:28:27 +0100 Jan Kara <[email protected]> wrote:

New description which should address above questions:
Background writeback is easily livelockable in a loop in wb_writeback() by a process continuously re-dirtying pages (or continuously appending to a file). This is in fact intended as the target of background writeback is to write dirty pages it can find as long as we are over dirty_background_threshold.

Well. The objective of the kupdate function is utterly different.

Quoted 3 lines

But the above behavior gets inconvenient at times because no other work queued in the flusher thread's queue gets processed. In particular, since e.g. sync(1) relies on flusher thread to do all the IO for it,

That's fixable by doing the work synchronously within sync_inodes_sb(), rather than twiddling thumbs wasting a thread resource while waiting for kernel threads to do it. As an added bonus, this even makes cpu time accounting more accurate ;)

Quoted 4 lines

Quoted 2 lines

Please remind me why we decided to hand the sync_inodes_sb() work off to other threads?

Because when sync(1) does IO on it's own, it competes for the device with
the flusher thread running in parallel thus resulting in more seeks.

Skeptical. Has that effect been demonstrated? Has it been shown to be a significant problem? A worse problem than livelocking the machine? ;)

If this _is_ a problem then it's also a problem for fsync/msync. But see below.

Quoted 45 lines

Quoted 11 lines

Quoted 11 lines

sync(1) can hang forever waiting for flusher thread to do the work.

Generally, when a flusher thread has some work queued, someone submitted the work to achieve a goal more specific than what background writeback does. Moreover by working on the specific work, we also reduce amount of dirty pages which is exactly the target of background writeout. So it makes sense to give specific work a priority over a generic page cleaning.

Thus we interrupt background writeback if there is some other work to do. We return to the background writeback after completing all the queued work.

...

Quoted 11 lines

Quoted 10 lines

Quoted 2 lines

So... what prevents higher priority works (eg, sync(1)) from livelocking or seriously retarding background or kudate writeout?

If other work than background or kupdate writeout livelocks, it's a bug
which should be fixed (either by setting sensible nr_to_write or by tagging like we do it for WB_SYNC_ALL writeback). Of course, higher priority work can be running when background or kupdate writeout would need to run as well. But the idea here is that the purpose of background/kupdate types of writeout is to get rid of dirty data and any type of writeout does this so working on it we also work on background/kupdate writeout only possibly less efficiently.

Quoted 7 lines

The kupdate function is a data-integrity/quality-of-service sort of thing.

And what I'm asking is whether this change enables scenarios in which these threads can be kept so busy that the kupdate function gets interrupted so frequently that we can have dirty memory not being written back for arbitrarily long periods of time?

So let me compare:
What kupdate writeback does:
queue inodes older than dirty_expire_centisecs while some inode in the queue
write MAX_WRITEBACK_PAGES from each inode queued break if nr_to_write <= 0

What any other WB_SYNC_NONE writeback (let me call it "normal WB_SYNC_NONE writeback") does:
queue all dirty inodes
while some inode in the queue
write MAX_WRITEBACK_PAGES from each inode queued
break if nr_to_write <= 0

Quoted 2 lines

There only one kind of WB_SYNC_ALL writeback - the one which writes everything.

fsync() uses WB_SYNC_ALL and it doesn't write "everything".

Quoted 4 lines

So after WB_SYNC_ALL writeback (provided all livelocks are fixed ;) obviously no old data should be unwritten in memory. Normal WB_SYNC_NONE writeback differs from a kupdate one *only* in the fact that we queue all inodes instead of only the old ones.

whoa.

That's only true for sync(), or sync_filesystem(). It isn't true for, say, fsync() and msync(). Now when someone comes along and changes fsync/msync to use flusher threads ("resulting in less seeks") then we could get into a situation where fsync-serving flusher threads never serve kupdate?

Quoted 1 line

We start writing old inodes first and

OT, but: your faith in those time-ordered inode lists is touching ;) Put a debug function in there which checks that the lists _are_ time-ordered, and call that function from every site in the kernel
which modifies the lists. I bet there are still gremlins.

From: Jan Kara <[email protected]>

When wb_writeback() is called in WB_SYNC_ALL mode, work->nr_to_write is usually set to LONG_MAX. The logic in wb_writeback() then calls __writeback_inodes_sb() with nr_to_write == MAX_WRITEBACK_PAGES and thus we easily end up with negative nr_to_write after the function returns.

wb_writeback() then decides we need another round of writeback but this is wrong in some cases! For example when a single large file is continuously dirtied, we would never finish syncing it because each pass would be able to write MAX_WRITEBACK_PAGES and inode dirty timestamp never gets updated (as inode is never completely clean).

Fix the issue by setting nr_to_write to LONG_MAX in WB_SYNC_ALL mode. We do not need nr_to_write in WB_SYNC_ALL mode anyway since livelock avoidance is done differently for it.

Quoted 2 lines · On Tue, 09 Nov 2010 07:09:20 +0800 Wu Fengguang <[email protected]> wrote:

From: Jan Kara <[email protected]>

Quoted 4 lines

When wb_writeback() is called in WB_SYNC_ALL mode, work->nr_to_write is usually set to LONG_MAX. The logic in wb_writeback() then calls __writeback_inodes_sb() with nr_to_write == MAX_WRITEBACK_PAGES and thus we easily end up with negative nr_to_write after the function returns.

No, nr_to_write can only be negative if the filesystem wrote back more pages than requested.

Quoted 5 lines

wb_writeback() then decides we need another round of writeback but this is wrong in some cases! For example when a single large file is continuously dirtied, we would never finish syncing it because each pass would be able to write MAX_WRITEBACK_PAGES and inode dirty timestamp never gets updated (as inode is never completely clean).

Well we shouldn't have asked the function to write LONG_MAX pages then!

The way this used to work was to try to write back N=(total dirty pages
+ total unstable pages + various fudge factors) to each superblock. So each superblock will get fully written back unless someone is madly writing to it. If that _is_ happening then we'll write a large amount of data to it and will then give up and move onto the next superblock.

But the "large amount of data" is constrained to a sane upper limit:
total amount of dirty memory plus fudge factors. Increasing that sane upper limit to an insane 2^63-1 pages will *of course* cause sync() to livelock.

Why was that sane->insane change made?

Quoted 3 lines

Fix the issue by setting nr_to_write to LONG_MAX in WB_SYNC_ALL mode. We do not need nr_to_write in WB_SYNC_ALL mode anyway since livelock avoidance is done differently for it.

Here the changelog should spell out what "done differently" means. Because I really am unsure what is begin referred to.

I don't really see how this patch changes anything. For WB_SYNC_ALL requests the code will still try to write out 2^63 pages, only it does it all in a single writeback_inodes_wb() call. What prevents that call itself from getting livelocked?

Perhaps the unmentioned problem here is that each call to writeback_inodes_wb(MAX_WRITEBACK_PAGES) will restart its walk across the inode lists. So instead of giving up on a being-written-to-file, we continuously revisit it again and again and again.

Correct? If so, please add the description. If incorrect, please add the description as well ;)

Root cause time: it's those damn per-sb inode lists *again*. They're just awful. We need some data structure there which is more amenable to being iterated over. Something against which we can store cursors, for a start.

Quoted 3 lines · Andrew Morton

I don't really see how this patch changes anything. For WB_SYNC_ALL requests the code will still try to write out 2^63 pages, only it does it all in a single writeback_inodes_wb() call. What prevents that call

Quoted 1 line

itself from getting livelocked?

I'm referring to the livelock avoidance using page tagging. Fengguang
actually added a note about this into a comment in the code but it's not in the changelog. And you're right it should be here.

Quoted 7 lines

Perhaps the unmentioned problem here is that each call to writeback_inodes_wb(MAX_WRITEBACK_PAGES) will restart its walk across the inode lists. So instead of giving up on a being-written-to-file, we continuously revisit it again and again and again.

Correct? If so, please add the description. If incorrect, please add the description as well ;)

Yes, that's the problem.

Quoted 4 lines

Root cause time: it's those damn per-sb inode lists *again*. They're just awful. We need some data structure there which is more amenable to being iterated over. Something against which we can store cursors, for a start.

This would be definitely nice. But in this particular case, since we have
that page tagging livelock avoidance, we can just do all we need in a one big sweep so we are OK.

Suggestion for the new changelog:
When wb_writeback() is called in WB_SYNC_ALL mode, work->nr_to_write is usually set to LONG_MAX. The logic in wb_writeback() then calls __writeback_inodes_sb() with nr_to_write == MAX_WRITEBACK_PAGES and

we easily end up with negative nr_to_write after the function returns. This is because write_cache_pages() does not stop writing when nr_to_write drops to zero in WB_SYNC_ALL mode.

In WB_SYNC_ALL mode, filesystems are not expected to skip dirty pages on temporal lock contentions or non fatal errors, otherwise sync() will return without actually syncing the skipped pages. Add a check to catch possible redirty_page_for_writepage() callers that violate this expectation.

I'd recommend to keep this check in -mm tree for some time and fixup the possible warnings before pushing it to upstream.

Signed-off-by Wu Fengguang <[email protected]>

---
 fs/fs-writeback.c |    1 +
 1 file changed, 1 insertion(+)

Quoted 17 lines · On Tue, 09 Nov 2010 07:09:21 +0800 Wu Fengguang <[email protected]> wrote:

In WB_SYNC_ALL mode, filesystems are not expected to skip dirty pages on temporal lock contentions or non fatal errors, otherwise sync() will return without actually syncing the skipped pages. Add a check to catch possible redirty_page_for_writepage() callers that violate this expectation.

I'd recommend to keep this check in -mm tree for some time and fixup the possible warnings before pushing it to upstream.

Signed-off-by Wu Fengguang <[email protected]>

---
 fs/fs-writeback.c |    1 +
 1 file changed, 1 insertion(+)

fs/fs-writeback.c static int writeback_sb_inodes(struct su · line 533

Quoted 7 lines

fs/fs-writeback.c +1 -0

@@ -527,6 +527,7 @@ static int writeback_sb_inodes(struct su
 			 * buffers.  Skip this inode for now.
 			 */
 			redirty_tail(inode);
+			WARN_ON_ONCE(wbc->sync_mode == WB_SYNC_ALL);
 		}
 		spin_unlock(&inode_lock);
 		iput(inode);

This is quite kernel-developer-unfriendly.

Suppose the warning triggers. Now some poor schmuck looks at the warning and doesn't have a *clue* why it was added. He has to run off and grovel through git trees finding changelogs, which is a real pain if the code has been trivially altered since it was first added.

As a general rule, a kernel developer should be able to look at a warning callsite and then work out why the warning was emitted!

IOW, you owe us a code comment, please.

Andrew,

Here are the two writeback livelock fixes (patch 3, 4) from Jan Kara. The series starts with a preparation patch and carries two more debugging patches.