[Top] [All Lists]

Re: [PATCH 1/2] xfs: add CRC infrastructure

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 1/2] xfs: add CRC infrastructure
From: Mark Tinguely <tinguely@xxxxxxx>
Date: Fri, 09 Nov 2012 16:09:30 -0600
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1352295452-4726-2-git-send-email-david@xxxxxxxxxxxxx>
References: <1352295452-4726-1-git-send-email-david@xxxxxxxxxxxxx> <1352295452-4726-2-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:9.0) Gecko/20120122 Thunderbird/9.0
On 11/07/12 07:37, Dave Chinner wrote:
From: Christoph Hellwig<hch@xxxxxx>

  - add a mount feature bit for CRC enabled filesystems
  - add some helpers for generating and verifying the CRCs
  - add a copy_uuid helper

The checksumming helpers are losely based on similar ones in sctp,
all other bits come from Dave Chinner.

Signed-off-by: Christoph Hellwig<hch@xxxxxx>
Signed-off-by: Dave Chinner<dchinner@xxxxxxxxxx>
+ * 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.
+ */
+static inline __uint32_t
+xfs_start_cksum(char *buffer, size_t length, unsigned long cksum_offset)
+       __uint32_t zero = 0;
+       __uint32_t crc;
+       /* Calculate CRC up to the checksum. */
+       crc = crc32c(XFS_CRC_SEED, buffer, cksum_offset);
+       /* Skip checksum field */
+       crc = crc32c(crc,&zero, sizeof(__u32));

I know this came from sctp and I know I can't convince you to copy/null
the *cksum_offset to make one block for those with hardware crc32c devices.

Since the *cksum_offset value is never used in creating and verifying
the checksum, the 4 bytes of zeros does not add any new information,
why not just drop it from the cksum calculation?

+       /* Calculate the rest of the CRC. */
+       return crc32c(crc,&buffer[cksum_offset + sizeof(__be32)],
+                     length - (cksum_offset + sizeof(__be32)));
+ * Convert the intermediate checksum to the final ondisk format.
+ *
+ * Note that crc32c is already endianess agnostic, so no additional
+ * byte swap is needed.
+ */
+static inline __be32
+xfs_end_cksum(__uint32_t crc)
+       return (__force __be32)~crc;

Wouldn't you have to cpu_to_le32() for big endian machines?

commit 9c5ff5f75d0d0a1c7928ecfae3f38418b51a88e3
Author: Vlad Yasevich <vladislav.yasevich@xxxxxx>
Date:   Thu Jan 22 14:52:23 2009 -0800

    sctp: Fix crc32c calculations on big-endian arhes.

    crc32c algorithm provides a byteswaped result.  On little-endian
    arches, the result ends up in big-endian/network byte order.
    On big-endinan arches, the result ends up in little-endian
    order and needs to be byte swapped again.  Thus calling cpu_to_le32
    gives the right output.

    Tested-by: Jukka Taimisto <jukka.taimisto@xxxxxxxxxxxxxx>
    Signed-off-by: Vlad Yasevich <vladislav.yasevich@xxxxxx>
    Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>

diff --git a/include/net/sctp/checksum.h b/include/net/sctp/checksum.h
index b799fb2..2fec3c3 100644
--- a/include/net/sctp/checksum.h
+++ b/include/net/sctp/checksum.h
@@ -79,5 +79,5 @@ static inline __be32 sctp_update_cksum(__u8 *buffer, __u16 length, __be32 crc32)

 static inline __be32 sctp_end_cksum(__be32 crc32)
-       return ~crc32;
+       return (__force __be32)~cpu_to_le32((__force u32)crc32);

<Prev in Thread] Current Thread [Next in Thread>