Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758325Ab0FCI3a (ORCPT ); Thu, 3 Jun 2010 04:29:30 -0400 Received: from mail-fx0-f46.google.com ([209.85.161.46]:50171 "EHLO mail-fx0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757905Ab0FCI31 convert rfc822-to-8bit (ORCPT ); Thu, 3 Jun 2010 04:29:27 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=googlemail.com; s=gamma; h=mime-version:reply-to:in-reply-to:references:from:date:message-id :subject:to:cc:content-type:content-transfer-encoding; b=mKIZhEk92fd2suoy/BwXP1acrCn6f3QMu4PIL9At4Y08DGI0NLjg0Po36Ldu148pnR kgWwPe6zO+SRJ7Y0kRCC3uIeLubNSbRByCY/ggX3neeYYoq93HC6pDjM85EmCP4893Bc 0v4Qwm8hRJu1W7H9La30J2BMxLG/TY1ADGArY= MIME-Version: 1.0 Reply-To: mtk.manpages@gmail.com In-Reply-To: References: <20100524070552.GR23411@kernel.dk> <20100524173551.GU23411@kernel.dk> <20100524175649.GV23411@kernel.dk> <20100601074534.GL1660@kernel.dk> <20100603061039.GD3564@kernel.dk> <20100603070126.GJ3564@kernel.dk> From: Michael Kerrisk Date: Thu, 3 Jun 2010 10:29:04 +0200 Message-ID: Subject: Re: [patch] pipe: add support for shrinking and growing pipes To: Jens Axboe Cc: OGAWA Hirofumi , Andrew Morton , Linus Torvalds , Miklos Szeredi , "linux-fsdevel@vger.kernel.org" , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6099 Lines: 133 Hi Jens, On Thu, Jun 3, 2010 at 9:58 AM, Michael Kerrisk wrote: > Hi Jens, > > On Thu, Jun 3, 2010 at 9:48 AM, Michael Kerrisk > wrote: >> On Thu, Jun 3, 2010 at 9:05 AM, Michael Kerrisk >> wrote: >>> On Thu, Jun 3, 2010 at 9:01 AM, Jens Axboe wrote: >>>> On Thu, Jun 03 2010, Michael Kerrisk wrote: >>>>> Hi Jens, >>>>> >>>>> On Thu, Jun 3, 2010 at 8:10 AM, Jens Axboe wrote: >>>>> > On Wed, Jun 02 2010, Michael Kerrisk wrote: >>>>> >> On Tue, Jun 1, 2010 at 9:45 AM, Jens Axboe wrote: >>>>> >> > On Thu, May 27 2010, Michael Kerrisk wrote: >>>>> >> >> Jens, >>>>> >> >> >>>>> >> >> On Mon, May 24, 2010 at 7:56 PM, Jens Axboe wrote: >>>>> >> >> > On Mon, May 24 2010, Michael Kerrisk wrote: >>>>> >> >> >> On Mon, May 24, 2010 at 7:35 PM, Jens Axboe wrote: >>>>> >> >> >> > On Mon, May 24 2010, Michael Kerrisk wrote: >>>>> >> >> >> >> > Right, that looks like a thinko. >>>>> >> >> >> >> > >>>>> >> >> >> >> > I'll submit a patch changing it to bytes and the agreed API and fix this >>>>> >> >> >> >> > -Eerror. Thanks for your comments and suggestions! >>>>> >> >> >> >> >>>>> >> >> >> >> Thanks. And of course you are welcome. (Please CC linux-api@vger on >>>>> >> >> >> >> this patche (and all patches that change the API/ABI.) >>>>> >> >> >> > >>>>> >> >> >> > The first change is this: >>>>> >> >> >> > >>>>> >> >> >> > http://git.kernel.dk/?p=linux-2.6-block.git;a=commit;h=0191f8697bbdfefcd36e7b8dc3eeddfe82893e4b >>>>> >> >> >> > >>>>> >> >> >> > and the one dealing with the pages vs bytes API is this: >>>>> >> >> >> > >>>>> >> >> >> > http://git.kernel.dk/?p=linux-2.6-block.git;a=commit;h=b9598db3401282bb27b4aef77e3eee12015f7f29 >>>>> >> >> >> > >>>>> >> >> >> > Not tested yet, will do so before sending in of course. >>>>> >> >> >> >>>>> >> >> >> Eyeballing it quickly, these changes look right. >>>>> >> >> > >>>>> >> >> > Good, thanks. >>>>> >> >> > >>>>> >> >> >> Do you have some test programs you can make available? >>>>> >> >> > >>>>> >> >> > Actually I don't, I test it by modifying fio's splice engine to set/get >>>>> >> >> > the pipe size and test the resulting transfers. >>>>> >> >> >>>>> >> >> An afterthought. Do there not also need to be fixes to the /proc >>>>> >> >> interfaces. I don't think they were included in your revised patches. >>>>> >> > >>>>> >> > I think the proc part can be sanely left in pages, since it's just a >>>>> >> > memory limiter. >>>>> >> >>>>> >> I can't see any advantage to using two different units for these >>>>> >> closely related APIs, and it does seem like it could be a source of >>>>> >> confusion. Similar APIs that I can think of like RLIMIT_MEMLOCK and >>>>> >> shmget() SHMMAX that impose per-process memory-related limits use >>>>> >> bytes. Best to be consistent, don't you think? >>>>> > >>>>> > But they are different interfaces. ?I think the 'pass in required size, >>>>> > return actual size' where actual size is >= required size makes sense >>>>> > for the syscall part, but for an "admin" interface it is more logical to >>>>> > deal in pages. Perhaps that's just me and the average admin does not >>>>> > agree. So while it's just detail, it's also an interface so has some >>>>> > importance. And if there's consensus that bytes is a cleaner interface >>>>> > on the proc side as well, then lets change it. >>>>> >>>>> I'll add one more datapoint to those that I already mentioned. >>>>> RLIMIT_STACK and RLIMIT_DATA (getrlimit()) is also expressed in bytes. >>>>> >>>>> There was only one vaguely related limit that I could find that >>>>> measured things in pages. Consider these two System V shared memory >>>>> limits: >>>>> >>>>> SHMMAX >>>>> This is the maximum size (in bytes) of a shared memory segment. >>>>> >>>>> SHMALL >>>>> This is a system-wide limit on the total number of pages of shared memory. >>>>> >>>>> But in a way this almost confirms my point. SHMMAX is a limit the >>>>> governs the behavior of individual processes (like your /proc file), >>>>> while SHMALL is a limit that governs the behavior of the system as a >>>>> whole. There is a (sort of) logic to using bytes for one and pages for >>>>> the other. >>>>> >>>>> I think that I've said all I need to say on the topic. I'm inclined to >>>>> think yours /proc file should use bytes, since it seems consistent >>>>> with other simialr APIs. Others may confirm, or someone else mught >>>>> have a different insight. >>>> >>>> I'll commit a patch to change it to bytes. >>> >>> Thanks Jens. >> >> Since I'm going to document the /proc file, it occurred to me... What >> are you going to call this file now? "pipe_max_pages" ?no longer makes >> sense. "pipe_size_ceiling" may be more expressive than simply >> "pipe_max". > > So, I'm looking at this interface still more closely now. How about > using CAP_SYS_RESOURCE, rather than the hugely overloaded > CAP_SYS_ADMIN as the governor for the capability check? Again, it's > about consistency. Here's what CAP_SYS_RESOURCE currently governs: > > ? ? ? CAP_SYS_RESOURCE > ? ? ? ? ? ? ?* Use reserved space on ext2 file systems; > ? ? ? ? ? ? ?* make ioctl(2) calls controlling ext3 journaling; > ? ? ? ? ? ? ?* override disk quota limits; > ? ? ? ? ? ? ?* increase resource limits (see setrlimit(2)); > ? ? ? ? ? ? ?* override RLIMIT_NPROC resource limit; > ? ? ? ? ? ? ?* raise msg_qbytes limit for a System V message queue > above the limit > ? ? ? ? ? ? ? ?in /proc/sys/kernel/msgmnb (see msgop(2) and msgctl(2)). > > Including the pipe size limit in this list makes sense. Another question: What happens if we adjust the capacity of a pipe to a value that is smaller than the number of bytes currently in the pipe? Cheers, Michael -- 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/