[Cluster-devel] [PATCH v2 0/2] fix gfs2 truncate race

[Benjamin Marzinski <bmarzins@redhat.com>]

If gfs2 tries to write out a page of a file with data journaling
enabled, while that file is being truncated, it can cause a kernel
panic. The problem is that the page it is writing out may point to past
the end of the file. If this happens, gfs2 will try to invalidate the
page.  However, it can't invalidate a journaled page with buffers in the
in-memory log, without revoking those buffers, so that there is no
chance of corruption if the machine crashes and later has its journal
replayed.  If the page is being written out by the log flush code, there
is no way that it can add a revoke entry onto the log during the flush.

To solve this, gfs2 simply writes out journalled data pages that point
past the end of the file, since the truncate is still in progress, and
everything will be cleaned up before the file is unlocked, or the blocks
are marked free. Doing this involves both a gfs2 change and exporting an
additional symbol from the vfs.

These v2 patches leave the code in gfs2_writepage_common alone, and
replace that call in gfs2_jdata_writepage with the necessary checks.  Doing
this highlighted the fact that __gfs2_jdata_writepage will never actually
run during a transaction, and unless starting and then ending an untouched
transaction has some important side effect that I can't see. All the
transaction code can be pulled out of gfs2_jdata_writepage with no changes
to how gfs2 is currently operating.

However, looking through that commit history, I don't think that this is
intentional.  It looks more like gfs2_jdata_writepage lost the ability to
usefully start transactions, and write out the page during them, so as to
allow it to invalidate the page in those cases.  Restoring this won't solve
the truncate race bug, since this can happen at times when gfs2_jdata_writepage
won't (and can't) start a transaction.

Steve, it looks like your commit 1bb7322fd0d5abdce396de51cbc5dbc489523018
caused this change. Do you have any idea if it was intentional. Clearly it
isn't breaking things. But we should either remove the transaction completely
like this patch, or make it possible to actually write out the page during
the transaction.

Benjamin Marzinski (2):
  fs: export __block_write_full_page
  gfs2: writeout truncated pages

 fs/buffer.c                 |  3 ++-
 fs/gfs2/aops.c              | 49 +++++++++++++++++++++++++++++++--------------
 include/linux/buffer_head.h |  3 +++
 3 files changed, 39 insertions(+), 16 deletions(-)

-- 
1.8.3.1