Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754784Ab2FNJgq (ORCPT ); Thu, 14 Jun 2012 05:36:46 -0400 Received: from mail-gg0-f174.google.com ([209.85.161.174]:56523 "EHLO mail-gg0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752184Ab2FNJgn convert rfc822-to-8bit (ORCPT ); Thu, 14 Jun 2012 05:36:43 -0400 MIME-Version: 1.0 In-Reply-To: <20120614072831.63845f6ddf024ece28e49f5e@gmail.com> References: <20120613130054.b5695621.yoshikawa.takuya@oss.ntt.co.jp> <20120613130304.0186fa9d.yoshikawa.takuya@oss.ntt.co.jp> <20120613214157.0a5179d5358ec7f1b2646606@gmail.com> <20120613230013.3cc59bf908616e94bb4ccef2@gmail.com> <20120614072831.63845f6ddf024ece28e49f5e@gmail.com> Date: Thu, 14 Jun 2012 18:36:42 +0900 Message-ID: Subject: Re: [PATCH 2/5] drivers/net/ethernet/dec/tulip: Use standard __set_bit_le() function From: Akinobu Mita To: Takuya Yoshikawa Cc: Grant Grundler , Takuya Yoshikawa , akpm@linux-foundation.org, bhutchings@solarflare.com, grundler@parisc-linux.org, arnd@arndb.de, benh@kernel.crashing.org, avi@redhat.com, mtosatti@redhat.com, linux-net-drivers@solarflare.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, kvm@vger.kernel.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1932 Lines: 46 2012/6/14 Takuya Yoshikawa : > On Wed, 13 Jun 2012 08:21:18 -0700 > Grant Grundler wrote: > >> >> >> Should this hash_table be converted from u16 hash_table[32] to >> >> >> DECLARE_BITMAP(hash_table, 16 * 32) to ensure that it is aligned >> >> >> on long-word boundary? >> >> > >> >> > I think hash_table is already long-word aligned because it is placed >> >> > right after a pointer. >> >> >> >> I recommend converting to proper bitmap. ?Because such an implicit >> >> assumption is easily broken by someone touching this function. >> > >> > Do you mean something like: >> > ? ? ? ?DECLARE_BITMAP(__hash_table, 16 * 32); >> > ? ? ? ?u16 *hash_table = (u16 *)__hash_table; >> > ? >> > >> > Grant, what do you think about this? >> >> Hi Takuya, >> two thoughts: >> 1) while I agree with Akinobu and thank him for pointing out a >> _potential_ alignment problem, this is a separate issue and your >> existing patch should go in anyway. There are probably other drivers >> with _potential_ alignment issues. Akinobu could get credit for >> finding them by submitting patches after reviewing calls to set_bit >> and set_bit_le() - similar to what you are doing now. > > I prefer approach 1. > > hash_table is local in build_setup_frame_hash(), so if further > improvement is also required, we can do that locally there later. This potential alignment problem is introduced by this patch. Because the original set_bit_le() in tulip driver can handle unaligned bitmap. This is why I recommended it should be fixed in this patch. But please just ignore me if I'm too much paranoid. And I'll handle this issue if no one wants to do it. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/