From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Snook Subject: Re: [RFC PATCH 0/2] avoid clobbering registers with J_ASSERT macro Date: Mon, 20 Aug 2007 09:18:21 -0400 Message-ID: <46C9949D.8060000@redhat.com> References: <46C5380A.8080109@redhat.com> <20070817135402.d7246766.akpm@linux-foundation.org> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Stephen Tweedie , linux-fsdevel@vger.kernel.org To: Andrew Morton Return-path: Received: from mx1.redhat.com ([66.187.233.31]:39211 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751953AbXHTNSZ (ORCPT ); Mon, 20 Aug 2007 09:18:25 -0400 In-Reply-To: <20070817135402.d7246766.akpm@linux-foundation.org> Sender: linux-fsdevel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org Andrew Morton wrote: > On Fri, 17 Aug 2007 01:54:18 -0400 > Chris Snook wrote: > >> The J_ASSERT() macro in jbd and jbd2 calls printk() prior to BUG(). While this >> makes it more convenient to read the assertion failure, it also clobbers >> registers, which can sometimes make debugging harder, which is clearly not the >> intended purpose. I recently banged my head on this myself. >> >> The following patches to jbd and jbd2 enable the printk only if >> CONFIG_JBD[2]_DEBUG is set. Otherwise, it will simply BUG if the condition is >> violated. This way test kernels still get the benefit of the J_ASSERT printk, >> while production kernels, which come from a more stable source base where it's >> easier to trace line numbers back to specific lines of code, simply get the BUG, >> with all registers preserved. >> >> This is, of course, not the only way of fixing this problem, but it seems to be >> the least invasive way, which is why I'm proposing these patches. > > How's about we just remove that printk? Do > > #define J_ASSERT(e) BUG_ON(e)? > > The rest of the kernel seems to be able to cope with that... Perfectly fine with me. I proposed the conditional on the grounds that someone probably had a purpose for the original J_ASSERT macro at some point in history, but if that purpose is long since obsolete, let's just get rid of it. I'll repost. -- Chris