Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 30 Nov 2011 22:01:12 +0100
From:      Giovanni Trematerra <gianni@freebsd.org>
To:        Konstantin Belousov <kib@freebsd.org>
Cc:        src-committers@freebsd.org, peterjeremy@acm.org, John Baldwin <jhb@freebsd.org>, svn-src-all@freebsd.org, Attilio Rao <attilio@freebsd.org>, svn-src-head@freebsd.org, Jeff Roberson <jeff@freebsd.org>
Subject:   Re: svn commit: r226042 - in head/sys: kern sys
Message-ID:  <CACfq090D6CgH=hbLxGwMaBpAnui0ibfSMbqAsBGgXinEJQG7Hg@mail.gmail.com>
In-Reply-To: <201110051656.p95Gu6Cw020744@svn.freebsd.org>
References:  <201110051656.p95Gu6Cw020744@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Oct 5, 2011 at 6:56 PM, Konstantin Belousov <kib@freebsd.org> wrote=
:
> Author: kib
> Date: Wed Oct =A05 16:56:06 2011
> New Revision: 226042
> URL: http://svn.freebsd.org/changeset/base/226042
>
> Log:
> =A0Supply unique (st_dev, st_ino) value pair for the fstat(2) done on the=
 pipes.
>
> =A0Reviewed by: =A0jhb, Peter Jeremy <peterjeremy acm org>
> =A0MFC after: =A0 =A02 weeks
>
> Modified:
> =A0head/sys/kern/sys_pipe.c
> =A0head/sys/sys/pipe.h
>

Hi Konstantin,
unfortunately your commit introduces a performance penalty of about 3%
documented below in a real workload.
I guess that fstat(2) on the pipes is seldom used so we could just be lazy
and alloc a new unr number inside pipe_stat instead of during pipe creation=
.
In that case if an application doesn't need fstat(2) on the pipes it
won't be charged
the cost of alloc_unr/free_unr for the fake inode number.
The patch I propose, furthermore, fix a panic in the case alloc_unr
failed to allocate
a new unr number and return -1. This is because ino_t is unsigned and the t=
est
pipe_ino > 0 into pipeclose would be true, calling then free_unr when
it hasn't to.
The proposed patch was tested with a regression test code that you can find=
 here

http://www.trematerra.net/patches/pipe-fstat.c

Feel free to add it under tools/regression/pipe/

Here the proposed patch:

http://www.trematerra.net/patches/lazy_inoalloc.diff

Here the report of the benchmark:

Configuration
10.0-CURRENT updated to r22807.
kern.smp.disabled=3D1 in /boot/loader.conf
kernel config GENERIC without debugging options.

The first result of any test was discarded and not reported here.

here the result of three executions of
# make -s buildkernel
note that I managed to compile the same identical source files
for all the tests.

r22807 with r226042 reverted (time in seconds)
527, 527, 527

r22807 (time in seconds)
544, 544, 544

r22807M with the proposed patch (time in seconds)
527, 528, 528

ministat output:

x r22807 with r226042 reverted
+ r22807
* r22807M with the proposed patch
+--------------------------------------------------------------------------=
----+
|+    *                                                                    =
   x|
|*    *                                                                    =
   x|
||__A_M|                                                                   =
   A|
+--------------------------------------------------------------------------=
----+
    N           Min           Max        Median           Avg        Stddev
x   3           544           544           544           544             0
+   3           527           527           527           527             0
Difference at 95.0% confidence
	-17 +/- 0
	-3.125% +/- 0%
	(Student's t, pooled s =3D 0)
*   3           527           528           528     527.66667    0.57735027
Difference at 95.0% confidence
	-16.3333 +/- 0.925333
	-3.00245% +/- 0.170098%
	(Student's t, pooled s =3D 0.408248)

--
Gianni



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CACfq090D6CgH=hbLxGwMaBpAnui0ibfSMbqAsBGgXinEJQG7Hg>