Re: [patch] pipe: add support for shrinking and growing pipes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jun 3, 2010 at 9:05 AM, Michael Kerrisk
<mtk.manpages@xxxxxxxxxxxxxx> wrote:
> On Thu, Jun 3, 2010 at 9:01 AM, Jens Axboe <jaxboe@xxxxxxxxxxxx> wrote:
>> On Thu, Jun 03 2010, Michael Kerrisk wrote:
>>> Hi Jens,
>>>
>>> On Thu, Jun 3, 2010 at 8:10 AM, Jens Axboe <jaxboe@xxxxxxxxxxxx> wrote:
>>> > On Wed, Jun 02 2010, Michael Kerrisk wrote:
>>> >> On Tue, Jun 1, 2010 at 9:45 AM, Jens Axboe <axboe@xxxxxxxxx> wrote:
>>> >> > On Thu, May 27 2010, Michael Kerrisk wrote:
>>> >> >> Jens,
>>> >> >>
>>> >> >> On Mon, May 24, 2010 at 7:56 PM, Jens Axboe <jens.axboe@xxxxxxxxxx> wrote:
>>> >> >> > On Mon, May 24 2010, Michael Kerrisk wrote:
>>> >> >> >> On Mon, May 24, 2010 at 7:35 PM, Jens Axboe <jens.axboe@xxxxxxxxxx> 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".

Cheers,

Michael
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux