From mboxrd@z Thu Jan 1 00:00:00 1970 From: Linus Torvalds Subject: Re: [PATCH] fs: fix lock initialization Date: Wed, 6 Jul 2011 10:40:44 -0700 Message-ID: References: <87iprf3dgs.fsf@tucsk.pomaz.szeredi.hu> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, akpm@linux-foundation.org, stable@kernel.org To: Miklos Szeredi Return-path: In-Reply-To: <87iprf3dgs.fsf@tucsk.pomaz.szeredi.hu> Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org On Wed, Jul 6, 2011 at 3:33 AM, Miklos Szeredi wrot= e: > > locks_alloc_lock() assumed that the allocated struct file_lock is > already initialized to zero members. =A0This is only true for the fir= st > allocation of the structure, after reuse some of the members will hav= e > random values. Ugh. Code that depends on SLAB initializers is broken. The reason for those initializers are traditionally "better cache behavior" where you don't need to initialize everything at allocation time, but the whole concept is almost invariably a disaster. This is a prime example of it. I also doubt the cache value - what happens is that you tend to touch _part_ of the cachelines instead of just clearing the whole thing, which is quite often not at all more efficient. And even when you are able to avoid touching a cacheline entirely, most of the time it just moves the cache miss cost elsewhere to the user. There are some valid uses of initializers (eg code that depends on RCU freeing and very much is about not re-initializing a lock), but they really are about those kinds special cases. We should try to encourage people to limit initializers to *only* those kinds of things. Thanks for noticing, Linus