Re: [PATCH] [v3] warn on performance-impacting configs aka. TAINT_PERFORMANCE

[Dave Hansen]

On 08/22/2014 12:20 AM, Ingo Molnar wrote:
> Essentially all DEBUG_OBJECTS_* options are expensive, assuming 
> they are enabled, i.e. DEBUG_OBJECTS_ENABLE_DEFAULT=y.
> 
> Otherwise they should only be warned about if the debugobjects 
> boot option got enabled.
> 
> I.e. you'll need a bit of a runtime check for this one.

At that point, what do we print, and when do we print it?  We're not
saying that the config option should be disabled because it's really the
boot option plus the config option that is causing the problem.

I'll just put the DEBUG_OBJECTS_ENABLE_DEFAULT in here which is
analogous to what we're doing with SLUB_DEBUG_ON.

>> +static ssize_t performance_taint_read(struct file *file, char __user *user_buf,
>> +			size_t count, loff_t *ppos)
>> +{
>> +	int i;
>> +	int ret;
>> +	char *buf;
>> +	size_t buf_written = 0;
>> +	size_t buf_left;
>> +	size_t buf_len;
>> +
>> +	if (!ARRAY_SIZE(perfomance_killing_configs))
>> +		return 0;
>> +
>> +	buf_len = 1;
>> +	for (i = 0; i < ARRAY_SIZE(perfomance_killing_configs); i++)
>> +		buf_len += strlen(config_prefix) +
>> +			   strlen(perfomance_killing_configs[i]);
>> +	/* Add a byte for for each entry in the array for a \n */
>> +	buf_len += ARRAY_SIZE(perfomance_killing_configs);
>> +
>> +	buf = kmalloc(buf_len, GFP_KERNEL);
>> +	if (!buf)
>> +		return -ENOMEM;
>> +
>> +	buf_left = buf_len;
>> +	for (i = 0; i < ARRAY_SIZE(perfomance_killing_configs); i++) {
>> +		buf_written += snprintf(buf + buf_written, buf_left,
>> +					"%s%s\n", config_prefix,
>> +					perfomance_killing_configs[i]);
>> +		buf_left = buf_len - buf_written;
> 
> So, ARRAY_SIZE(performance_killing_configs) is written out four 
> times, a temporary variable would be in order I suspect.

If one of them had gone over 80 chars, I probably would have. :)  I put
one in anyway.

> Also, do you want to check buf_left and break out early from 
> the loop if it goes non-positive?

You're slowly inflating my patch for no practical gain. :)

Oh well, I put that in too.

I think I got the rest of the things addressed too.  v4 coming soon.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/