Re: [PATCH 01/14] sysctl: provide callback for write into ctl_table entry

[Sasha Levin <levinsasha928@xxxxxxxxx>]

On Sun, Apr 29, 2012 at 10:22 AM, Eric W. Biederman
<ebiederm@xxxxxxxxxxxx> wrote:
> Sasha Levin <levinsasha928@xxxxxxxxx> writes:
>
>> Provide a callback that will be called when writing to a ctl_table
>> entry after the user input has been validated.
>>
>> This will simplify user input checks since now it will be possible to
>> remove them out of the proc_handler.
>
> Ick No.
>
> You are simplifying things by taking updates out of locks, and
> introducing races.

Exactly twp of the patches (out of 14) are taking updates out of
locks. I'm quite sure that doing that in the ftrace case is perfectly
fine, and I'll take a second look at the sched-rt one since there
indeed might be a race caused due to the patch that I've missed.

If we figure out that both cases are wrong, the solution would be to
drop these two patches from the series. I have only simplified that
I've thought to be simple common cases, if I'm mistaken about these
two then they're out.

> Your naming of the callback "callback" is much too generic.

I'd name it write_successful_callback() if there was a point for that,
but seeing as there are no other callback types (and I don't see a
need at the moment for other callbacks either), I've just named it
"callback".

Either way, I'm perfectly fine with renaming it to whatever works for
the rest of the community.

> I think the current function call mechanism of sysctl can be improved
> but I don't think you have come up with the right combination of things.

I'm not trying to fix the entire function call mechanism, I'm just
trying to correct a negative pattern that has developed along time.

If it doesn't fit the bigger view of the future of sysctl function
calls, let me know what's that view is exactly and we'll see if this
patch series can work there. If there's something specific that's
bothering you about this series, let me know and I'll fix it. But
saying that it sucks since it doesn't solve all the issues in sysctl
function calls doesn't work for me.
--
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/