From patchwork Thu Oct 27 16:17:47 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicholas Piggin X-Patchwork-Id: 9486037 X-Mozilla-Keys: nonjunk Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on sandeen.net X-Spam-Level: X-Spam-Status: No, score=-6.7 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID autolearn=ham autolearn_force=no version=3.4.0 X-Spam-HP: BAYES_00=-1.9,DKIM_ADSP_CUSTOM_MED=0.001,DKIM_SIGNED=0.1, FREEMAIL_FORGED_FROMDOMAIN=0.196,FREEMAIL_FROM=0.001, HEADER_FROM_DIFFERENT_DOMAINS=0.001, RCVD_IN_DNSWL_HI=-5, RP_MATCHES_RCVD=-0.1, T_DKIM_INVALID=0.01 X-Original-To: sandeen@sandeen.net Delivered-To: sandeen@sandeen.net Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by sandeen.net (Postfix) with ESMTP id 3A370182E2 for ; Thu, 27 Oct 2016 11:18:00 -0500 (CDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750823AbcJ0QSC (ORCPT ); Thu, 27 Oct 2016 12:18:02 -0400 Received: from mail-pf0-f194.google.com ([209.85.192.194]:33611 "EHLO mail-pf0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750748AbcJ0QSC (ORCPT ); Thu, 27 Oct 2016 12:18:02 -0400 Received: by mail-pf0-f194.google.com with SMTP id i85so3035376pfa.0 for ; Thu, 27 Oct 2016 09:18:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:organization:mime-version :content-transfer-encoding; bh=/OY0ny0s5P73uyRUUjcDfMBhptF1C38hc51XWg8T+N4=; b=l8uBeUDkVeIFMagjPmO2hWWPzuX4Y3egVCgVus/5EI8T2jPNc0F0oD7CIJlpBCKOtz /GwsagSyhIbdA5skogn+DXoKskqpshXS85qrxzKk6J7QHKuVCSPVd2UR+eYXvZmZaUEp 2Z7JbLaJZIkiaQBbMDY6F6zoV7c4rhAl0iBsvjjreuOmilTRdlIU/omsSEAp95y52DqH QahGlVoiTPxmhoaec6bn6QDqSrU9lDfOXqCEdjg4L576PMLWUpJI6C/QqiIV4kZX0q9Q MW/l4HQkkgCQWiFz3NXSyCKeBBAoD/nq0usqUgKjrHrDKfp2CZCAka8pXAhKmpI5g8by JGew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:organization :mime-version:content-transfer-encoding; bh=/OY0ny0s5P73uyRUUjcDfMBhptF1C38hc51XWg8T+N4=; b=P5jbW/Tda5YU3qdeR9zGH5YdSBmFI63MzVAxgvXhc+ZpuHyxYWXeKliPT+4CNUZ5Gh gsWPQdPBPk/KB2bsG9ciPsvPv1btmo7De7QHa+p5FZPRulVYkqfuBlclzy7/XRa8UVFm h+fGhWi0ftWEsz848ERDrNe26Fm50qknDuVz5mLALR+/mg7vNP/RMgzBz85NannWSDKV fs0lyNR3sXXrduU5LN+MB8bhzb9jhAtP+kRZJFSGSwl3DthJ2bgRnyHBclSfXAixQUE1 KZxDU3pfSHBlht1q8CuF70FGLVe9lnFLWv60d/kR6KR8Td6Ku5M0DaGbkHgPEWk9Jd/o groA== X-Gm-Message-State: ABUngvcgDlE1Jd3auyQQxeSOUWJwKNbV9e3y0iKIfg5jOI3XwHWNgOeYyrR/QpG9edRdLw== X-Received: by 10.98.79.193 with SMTP id f62mr15780608pfj.94.1477585081000; Thu, 27 Oct 2016 09:18:01 -0700 (PDT) Received: from roar.ozlabs.ibm.com ([122.99.82.10]) by smtp.gmail.com with ESMTPSA id xn11sm12872389pac.38.2016.10.27.09.17.58 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 27 Oct 2016 09:18:00 -0700 (PDT) Date: Fri, 28 Oct 2016 03:17:47 +1100 From: Nicholas Piggin To: linux-xfs@vger.kernel.org Cc: Christoph Hellwig , Dave Chinner Subject: [rfc] larger batches for crc32c Message-ID: <20161028031747.68472ac7@roar.ozlabs.ibm.com> Organization: IBM X-Mailer: Claws Mail 3.14.0 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Sender: linux-xfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org Hi guys, We're seeing crc32c_le show up in xfs log checksumming on a MySQL benchmark on powerpc. I could reproduce similar overheads with dbench as well. 1.11% mysqld [kernel.vmlinux] [k] __crc32c_le | ---__crc32c_le | --1.11%--chksum_update | --1.11%--crypto_shash_update crc32c xlog_cksum xlog_sync _xfs_log_force_lsn xfs_file_fsync vfs_fsync_range do_fsync sys_fsync system_call 0x17738 0x17704 os_file_flush_func fil_flush As a rule, it helps the crc implementation if it can operate on as large a chunk as possible (alignment, startup overhead, etc). So I did a quick hack at getting XFS checksumming to feed crc32c() with larger chunks, by setting the existing crc to 0 before running over the entire buffer. Together with some small work on the powerpc crc implementation, crc drops below 0.1%. I don't know if something like this would be acceptable? It's not pretty, but I didn't see an easier way. diff --git a/fs/xfs/libxfs/xfs_cksum.h b/fs/xfs/libxfs/xfs_cksum.h index fad1676..88f4431 100644 --- a/fs/xfs/libxfs/xfs_cksum.h +++ b/fs/xfs/libxfs/xfs_cksum.h @@ -9,20 +9,9 @@ * cksum_offset parameter. */ static inline __uint32_t -xfs_start_cksum(char *buffer, size_t length, unsigned long cksum_offset) +xfs_start_cksum(void *buffer, size_t length) { - __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)); - - /* Calculate the rest of the CRC. */ - return crc32c(crc, &buffer[cksum_offset + sizeof(__be32)], - length - (cksum_offset + sizeof(__be32))); + return crc32c(XFS_CRC_SEED, buffer, length); } /* @@ -42,22 +31,29 @@ xfs_end_cksum(__uint32_t crc) * Helper to generate the checksum for a buffer. */ static inline void -xfs_update_cksum(char *buffer, size_t length, unsigned long cksum_offset) +xfs_update_cksum(void *buffer, size_t length, unsigned long cksum_offset) { - __uint32_t crc = xfs_start_cksum(buffer, length, cksum_offset); + __le32 *crcp = buffer + cksum_offset; - *(__le32 *)(buffer + cksum_offset) = xfs_end_cksum(crc); + *crcp = 0; /* set to 0 before calculating checksum */ + *crcp = xfs_end_cksum(xfs_start_cksum(buffer, length)); } /* * Helper to verify the checksum for a buffer. */ static inline int -xfs_verify_cksum(char *buffer, size_t length, unsigned long cksum_offset) +xfs_verify_cksum(void *buffer, size_t length, unsigned long cksum_offset) { - __uint32_t crc = xfs_start_cksum(buffer, length, cksum_offset); + __le32 *crcp = buffer + cksum_offset; + __le32 crc = *crcp; + __le32 new_crc; + + *crcp = 0; + new_crc = xfs_end_cksum(xfs_start_cksum(buffer, length)); + *crcp = crc; /* restore original CRC in place */ - return *(__le32 *)(buffer + cksum_offset) == xfs_end_cksum(crc); + return new_crc == crc; } #endif /* _XFS_CKSUM_H */ diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c index 8de9a3a..efd8daa 100644 --- a/fs/xfs/libxfs/xfs_inode_buf.c +++ b/fs/xfs/libxfs/xfs_inode_buf.c @@ -419,15 +419,11 @@ xfs_dinode_calc_crc( struct xfs_mount *mp, struct xfs_dinode *dip) { - __uint32_t crc; - if (dip->di_version < 3) return; ASSERT(xfs_sb_version_hascrc(&mp->m_sb)); - crc = xfs_start_cksum((char *)dip, mp->m_sb.sb_inodesize, - XFS_DINODE_CRC_OFF); - dip->di_crc = xfs_end_cksum(crc); + xfs_update_cksum(dip, mp->m_sb.sb_inodesize, XFS_DINODE_CRC_OFF); } /* diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c index 3b74fa0..4e242f0 100644 --- a/fs/xfs/xfs_log.c +++ b/fs/xfs/xfs_log.c @@ -1658,7 +1658,7 @@ xlog_pack_data( * This is a little more complicated than it should be because the various * headers and the actual data are non-contiguous. */ -__le32 +void xlog_cksum( struct xlog *log, struct xlog_rec_header *rhead, @@ -1667,10 +1667,9 @@ xlog_cksum( { __uint32_t crc; - /* first generate the crc for the record header ... */ - crc = xfs_start_cksum((char *)rhead, - sizeof(struct xlog_rec_header), - offsetof(struct xlog_rec_header, h_crc)); + /* first generate the crc for the record header with 0 crc... */ + rhead->h_crc = 0; + crc = xfs_start_cksum(rhead, sizeof(struct xlog_rec_header)); /* ... then for additional cycle data for v2 logs ... */ if (xfs_sb_version_haslogv2(&log->l_mp->m_sb)) { @@ -1691,7 +1690,7 @@ xlog_cksum( /* ... and finally for the payload */ crc = crc32c(crc, dp, size); - return xfs_end_cksum(crc); + rhead->h_crc = xfs_end_cksum(crc); } /* @@ -1840,8 +1839,7 @@ xlog_sync( } /* calculcate the checksum */ - iclog->ic_header.h_crc = xlog_cksum(log, &iclog->ic_header, - iclog->ic_datap, size); + xlog_cksum(log, &iclog->ic_header, iclog->ic_datap, size); #ifdef DEBUG /* * Intentionally corrupt the log record CRC based on the error injection diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h index 2b6eec5..18ba385 100644 --- a/fs/xfs/xfs_log_priv.h +++ b/fs/xfs/xfs_log_priv.h @@ -432,7 +432,7 @@ xlog_recover_finish( extern int xlog_recover_cancel(struct xlog *); -extern __le32 xlog_cksum(struct xlog *log, struct xlog_rec_header *rhead, +extern void xlog_cksum(struct xlog *log, struct xlog_rec_header *rhead, char *dp, int size); extern kmem_zone_t *xfs_log_ticket_zone; diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c index 9b3d7c7..3f62d9a 100644 --- a/fs/xfs/xfs_log_recover.c +++ b/fs/xfs/xfs_log_recover.c @@ -5114,8 +5114,13 @@ xlog_recover_process( { int error; __le32 crc; + __le32 old_crc; - crc = xlog_cksum(log, rhead, dp, be32_to_cpu(rhead->h_len)); + old_crc = rhead->h_crc; + rhead->h_crc = 0; + xlog_cksum(log, rhead, dp, be32_to_cpu(rhead->h_len)); + crc = rhead->h_crc; + rhead->h_crc = old_crc; /* * Nothing else to do if this is a CRC verification pass. Just return