xfs: optimise CRC updates

Source kernel commit: cae028df53449905c944603df624ac94bc619661

Nick Piggin reported that the CRC overhead in an fsync heavy
workload was higher than expected on a Power8 machine. Part of this
was to do with the fact that the power8 CRC implementation is not
efficient for CRC lengths of less than 512 bytes, and so the way we
split the CRCs over the CRC field means a lot of the CRCs are
reduced to being less than than optimal size.

To optimise this, change the CRC update mechanism to zero the CRC
field first, and then compute the CRC in one pass over the buffer
and write the result back into the buffer. We can do this safely
because anything writing a CRC has exclusive access to the buffer
the CRC is being calculated over.

We leave the CRC verify code the same - it still splits the CRC
calculation - because we do not want read-only operations modifying
the underlying buffer. This is because read-only operations may not
have an exclusive access to the buffer guaranteed, and so temporary
modifications could leak out to to other processes accessing the
buffer concurrently.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Dave Chinner <david@fromorbit.com>
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>

diff --git a/libxfs/xfs_cksum.h b/libxfs/xfs_cksum.h
index fad1676ad8cd25cd892799cee07cd01d3009fdd0..a416c7cb23ea57ed44df6db0844f623beab1e4b7 100644 (file)
--- a/libxfs/xfs_cksum.h
+++ b/libxfs/xfs_cksum.h
@@ -6,10 +6,11 @@
 /*
  * Calculate the intermediate checksum for a buffer that has the CRC field
  * inside it.  The offset of the 32bit crc fields is passed as the
- * cksum_offset parameter.
+ * cksum_offset parameter. We do not modify the buffer during verification,
+ * hence we have to split the CRC calculation across the cksum_offset.
  */
 static inline __uint32_t
-xfs_start_cksum(char *buffer, size_t length, unsigned long cksum_offset)
+xfs_start_cksum_safe(char *buffer, size_t length, unsigned long cksum_offset)
 {
        __uint32_t zero = 0;
        __uint32_t crc;
@@ -25,6 +26,20 @@ xfs_start_cksum(char *buffer, size_t length, unsigned long cksum_offset)
                      length - (cksum_offset + sizeof(__be32)));
 }
 
+/*
+ * Fast CRC method where the buffer is modified. Callers must have exclusive
+ * access to the buffer while the calculation takes place.
+ */
+static inline __uint32_t
+xfs_start_cksum_update(char *buffer, size_t length, unsigned long cksum_offset)
+{
+       /* zero the CRC field */
+       *(__le32 *)(buffer + cksum_offset) = 0;
+
+       /* single pass CRC calculation for the entire buffer */
+       return crc32c(XFS_CRC_SEED, buffer, length);
+}
+
 /*
  * Convert the intermediate checksum to the final ondisk format.
  *
@@ -40,11 +55,14 @@ xfs_end_cksum(__uint32_t crc)
 
 /*
  * Helper to generate the checksum for a buffer.
+ *
+ * This modifies the buffer temporarily - callers must have exclusive
+ * access to the buffer while the calculation takes place.
  */
 static inline void
 xfs_update_cksum(char *buffer, size_t length, unsigned long cksum_offset)
 {
-       __uint32_t crc = xfs_start_cksum(buffer, length, cksum_offset);
+       __uint32_t crc = xfs_start_cksum_update(buffer, length, cksum_offset);
 
        *(__le32 *)(buffer + cksum_offset) = xfs_end_cksum(crc);
 }
@@ -55,7 +73,7 @@ xfs_update_cksum(char *buffer, size_t length, unsigned long cksum_offset)
 static inline int
 xfs_verify_cksum(char *buffer, size_t length, unsigned long cksum_offset)
 {
-       __uint32_t crc = xfs_start_cksum(buffer, length, cksum_offset);
+       __uint32_t crc = xfs_start_cksum_safe(buffer, length, cksum_offset);
 
        return *(__le32 *)(buffer + cksum_offset) == xfs_end_cksum(crc);
 }

diff --git a/libxfs/xfs_inode_buf.c b/libxfs/xfs_inode_buf.c
index 2e3f0d6df14f1c1b2d63abdb3499177318cb72a7..2acc8fdb51e8278d1b54f2eb80317a0c25d15769 100644 (file)
--- a/libxfs/xfs_inode_buf.c
+++ b/libxfs/xfs_inode_buf.c
@@ -442,7 +442,7 @@ xfs_dinode_calc_crc(
                return;
 
        ASSERT(xfs_sb_version_hascrc(&mp->m_sb));
-       crc = xfs_start_cksum((char *)dip, mp->m_sb.sb_inodesize,
+       crc = xfs_start_cksum_update((char *)dip, mp->m_sb.sb_inodesize,
                              XFS_DINODE_CRC_OFF);
        dip->di_crc = xfs_end_cksum(crc);
 }