From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753558Ab2D2MIW (ORCPT ); Sun, 29 Apr 2012 08:08:22 -0400 Received: from mail-iy0-f174.google.com ([209.85.210.174]:35070 "EHLO mail-iy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752525Ab2D2MIT (ORCPT ); Sun, 29 Apr 2012 08:08:19 -0400 MIME-Version: 1.0 In-Reply-To: References: <1335681937-3715-1-git-send-email-levinsasha928@gmail.com> From: Sasha Levin Date: Sun, 29 Apr 2012 14:07:58 +0200 Message-ID: Subject: Re: [PATCH 01/14] sysctl: provide callback for write into ctl_table entry To: "Eric W. Biederman" Cc: viro@zeniv.linux.org.uk, rostedt@goodmis.org, fweisbec@gmail.com, mingo@redhat.com, a.p.zijlstra@chello.nl, paulus@samba.org, acme@ghostprotocols.net, james.l.morris@oracle.com, akpm@linux-foundation.org, tglx@linutronix.de, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-security-module@vger.kernel.org Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Apr 29, 2012 at 10:22 AM, Eric W. Biederman wrote: > Sasha Levin 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.