xfs
[Top] [All Lists]

Re: [PATCH V2] xfs: introduce CONFIG_XFS_WARN

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH V2] xfs: introduce CONFIG_XFS_WARN
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Wed, 24 Apr 2013 19:21:17 -0500
Cc: Chandra Seetharaman <sekharan@xxxxxxxxxx>, Brian Foster <bfoster@xxxxxxxxxx>, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20130424225821.GV10481@dastard>
References: <1366699138-24055-1-git-send-email-david@xxxxxxxxxxxxx> <5176833C.8070305@xxxxxxxxxx> <20130423210226.GN10481@dastard> <20130424085546.GQ10481@dastard> <1366828792.3762.32405.camel@xxxxxxxxxxxxxxxxxx> <20130424225821.GV10481@dastard>
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:17.0) Gecko/20130328 Thunderbird/17.0.5
On 4/24/13 5:58 PM, Dave Chinner wrote:
> On Wed, Apr 24, 2013 at 01:39:52PM -0500, Chandra Seetharaman wrote:
>> Hi Dave,
>>
>> Since this solution is for production environment, would it be valuable
>> to have a sysctl variable to allow enabling/disabling XFS_WARN, as
>> opposed to needing to recompile the module afresh ?
> 
> The idea is that distros enable it on the debug kernel packages
> rather than using CONFIG_XFS_DEBUG=y. Hence if someone has a
> problem, they just install the debug kernel and they get all this
> additional checking.
> 
> There are roughly 1700 ASSERT statements in the XFS code, so
> compiling them in unconditionally is a lot of extra code. Adding a
> sysctl to make them conditional adds as many branches into the code
> than in 99.99999% of calls are never going to evaluate as true. The
> convenience of a sysctl is more than outweighed by the additional
> overhead for the majortiy of people that don't need to diagnose
> problems on their system.
> 
> Hence I don't think the overhead of unconditionally compiling in
> ASSERT checks is worth while for the majority of users, especially
> as most distros ship a debug kernel for exactly this purpose....

I agree.  kernel-debug, or whatever your distro does similarly,
is the right use for this code.

-Eric

> Cheers,
> 
> Dave.
> 

<Prev in Thread] Current Thread [Next in Thread>