Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754099Ab1FLPe7 (ORCPT ); Sun, 12 Jun 2011 11:34:59 -0400 Received: from out01.mta.xmission.com ([166.70.13.231]:49759 "EHLO out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754024Ab1FLPeu (ORCPT ); Sun, 12 Jun 2011 11:34:50 -0400 From: ebiederm@xmission.com (Eric W. Biederman) To: Kay Sievers Cc: Lucas De Marchi , Alan Cox , linux-kernel@vger.kernel.org, Nick Piggin , Al Viro , Christoph Hellwig , Stephen Rothwell , Andrew Morton , David Howells , "Serge E. Hallyn" , Daniel Lezcano , Jiri Slaby , Greg Kroah-Hartman , James Morris References: <1306930476-1899-1-git-send-email-lucas.demarchi@profusion.mobi> <20110602134338.0c56160e@lxorguk.ukuu.org.uk> Date: Sun, 12 Jun 2011 08:34:31 -0700 In-Reply-To: (Kay Sievers's message of "Thu, 2 Jun 2011 19:32:49 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-XM-SPF: eid=;;;mid=;;;hst=in02.mta.xmission.com;;;ip=98.207.153.68;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1/lpJpKAv7oKPo5q/bb/nzxhYYw/B9Ms1w= X-SA-Exim-Connect-IP: 98.207.153.68 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.0 T_TM2_M_HEADER_IN_MSG BODY: T_TM2_M_HEADER_IN_MSG * -3.0 BAYES_00 BODY: Bayes spam probability is 0 to 1% * [score: 0.0000] * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa03 1397; Body=1 Fuz1=1 Fuz2=1] * 0.5 XM_Body_Dirty_Words Contains a dirty word * 0.0 T_TooManySym_01 4+ unique symbols in subject * 0.1 XMSolicitRefs_0 Weightloss drug * 0.4 UNTRUSTED_Relay Comes from a non-trusted relay X-Spam-DCC: XMission; sa03 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ;Kay Sievers X-Spam-Relay-Country: Subject: Re: [PATCH] sysctl: add support for poll() X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Fri, 06 Aug 2010 16:31:04 -0600) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3892 Lines: 93 Kay Sievers writes: > On Thu, Jun 2, 2011 at 15:56, Eric W. Biederman wrote: >> Lucas De Marchi writes: > >>> With this patch in, if anyone wants to manage a file under /proc/sys >>> there's really a small amount of code to write. He only has to define >>> the new poll struct for that file. >> >> The support currently appears cumbersome to add, and it adds what appear >> to be unnecessary wake ups (say when the hostname in another uts >> namespace changes). > > Yeah, *possibly* waking up once a day compared to *unconditionally* > waking up every second in every namespace and check if something has > changed. If that *possibly* should be optimized, which I think isn't > necessary at all, I guess the logic could be moved down to the > namespaced data. Unless you happen to be on a system, where someone has decided that it has a daemon that likes creating a new set of namespaces per connection to reduce the effect of code bugs. At which point you could be talking much more than a wake up per second. >> There is no explanation at all of why you care about the nis domainname. > > Just the same reason as the hostname, we need to know when the > kernel's internal state changes. regardless who did it and why, system > services need to follow it. So the problem is that you have a system where userspace can't get it's act together? >> Since there does not appear to be a specific problem that this problem >> is being aimed at, since the code just looks like extra maintenance and >> since the code needed to support this appears to be unnecessarily >> cumbersome I am going to nack the patch for now. >> >> Nacked-by: "Eric W. Biederman" > > Please provide solid technical reasons, why we can't to the same we do > everywhere else since years, or provide working alternatives. Until > then: > > Acked-By: Kay Sievers Looking a little closer the patch is broken. It changes the return of poll for every file under /proc/sys reporting no file under /proc/sys is writable unless it implements the new poll method. Also with having the wait_queue owned by the calling code either I am mistake or this breaks hotplug type situations. How do you get things off of your wait queue when you remove a file from /proc/sys. This infrastructure as written looks like a setup for use after free problems. > I think poll() is the natural interface if you care about the data in > a file. It's the same an single fd you care, and not some iniotify > watch, fd, pathname to register, and filter for whatever comes out > there. Sort of. poll is really designed for socket and pipe data. The problem with poll is there is no POLLUPDATED. > We already use exactly the same semantics for quite some years for > /proc/mounts, /proc/swaps, /proc/mdstat, ... and all over /sys. The > patch just provides the missing pieces that /proc provides, but > /proc/sys is missing. Good point. There is still the problem that the infrastructure code for the proc sysctl files are in much worse shape than the sysfs files. > I don't disagree, it might be nice to have generic inotify support on > /proc for this or other problems. But unless you want to work on it > now, and it would solve these problems, I don't see how we can get the > functionality we need, and that seems to solved with this patch. > Besides the fact that I think poll() is the simple and better > solution. Which is a question that has never been answered. What functionality do you need? To me this all looks like a bad science experiment. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/