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

[Bob Peterson]

----- Original Message -----
| 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
| 
| 
Hi,

Thanks. These are now applied to the for-next branch of the linux-gfs2 tree:
https://git.kernel.org/cgit/linux/kernel/git/gfs2/linux-gfs2.git/commit/fs?h=for-next&id=b4bba38909c21689de21355e84259cb7b38f25ac
https://git.kernel.org/cgit/linux/kernel/git/gfs2/linux-gfs2.git/commit/fs?h=for-next&id=fd4c5748b8d3f7420e8932ed0bde3d53cc8acc9d

Regards,

Bob Peterson
Red Hat File Systems