From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752455AbdCORlm (ORCPT ); Wed, 15 Mar 2017 13:41:42 -0400 Received: from mx3-phx2.redhat.com ([209.132.183.24]:51802 "EHLO mx3-phx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751277AbdCORll (ORCPT ); Wed, 15 Mar 2017 13:41:41 -0400 Date: Wed, 15 Mar 2017 13:40:27 -0400 (EDT) From: Bob Peterson To: Linus Torvalds Cc: Andreas Gruenbacher , cluster-devel , Linux Kernel Mailing List Message-ID: <1503230067.2693820.1489599627064.JavaMail.zimbra@redhat.com> In-Reply-To: References: <186538212.2600624.1489588290616.JavaMail.zimbra@redhat.com> <1400281796.2601400.1489588358478.JavaMail.zimbra@redhat.com> Subject: Re: GFS2: pull request for high-priority bug MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [10.10.120.94] X-Mailer: Zimbra 8.0.6_GA_5922 (ZimbraWebClient - FF52 (Linux)/8.0.6_GA_5922) Thread-Topic: GFS2: pull request for high-priority bug Thread-Index: dh88Cj0+4JsZKrc//DIx5OlkjcCsrw== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org ----- Original Message ----- | On Wed, Mar 15, 2017 at 7:32 AM, Bob Peterson wrote: | > | > Andreas Gruenbacher (1): | > gfs2: Avoid alignment hole in struct lm_lockname | | So I've pulled this because I think it fixes a real bug, but honestly | I think it's the wrong fix. | | Marking that lm_lockname structure "packed, aligned(4)" means that the | compiler will now think that the 64-bit fields in it may be unaligned | - including on architectures where that can be very expensive and the | compiler now might generate stupid unaligned instruction sequences to | load those values. | | So the *correct* fix, I think, would have been: | | - add a comment about not having holes in the struct due to the hashing | | - sort the fields by size (so "ln_number" first, then "ln_sbd", then | "ln_type") | | - use offsetofend(struct lm_lockname, ln_type) instead of sizeof() when | hashing | | which avoids the "possibly generate garbage code" issue due to the | quick-and-dirty one-liner approach. | | Hmm? | | Linus Hi Linus, Thanks. Yes, good ideas. I see your point and I'll see if we can get that fixed up for the next merge window. Bob Peterson