Re: fix seekdir(3)
On Thu, Mar 6, 2014 at 10:36 AM, Ingo Schwarze schwa...@usta.de wrote: back in November 2013, following an idea by guenther@, i cooked up another optimization for seekdir(3), then failed to send out the patch. So here it is. ... With this patch, rewinddir(3) changes from always being the worst case to often being the best case (now always the best case for directories with less than about 500 entries, or for rewinding before reading less than about 500 entries), no other case becomes any worse, and some other, less important edge cases improve, too. What the patch does is: * Let opendir(3) save the cookie of the first entry of the directory. * Let seekdir(3) use that saved cookie to move the internal pointers to that entry when asked to do so, for example on rewinddir(3). * Let seekdir(3) update that saved cookie when the buffer is exhausted and when consequently, an lseek(2) had to be done. * Let seekdir(3) invalidate the buffer by setting the pointer after its end, instead of to its beginning. * Have readdir(3) refrain from invalidating the buffer when asked to return the first element from it. This comes with only one price: Adding another off_t to the opaque struct _dirdesc (= DIR), which already contains another off_t, two long, two int, and two pointers. That's not much growth, especially since no sane code will allocate large numbers of DIRs. As explained last year, this patch reduced typical execution times of rewinddir(3) on my notebook for about 100 microseconds to about 0.05 microseconds. The /usr/src/regress/lib/libc/telldir/ test suite is still happy, and so are some other, manual tests i have done. Looks good to me, with some nice simplifications thrown in. Am i right that, the type DIR being opaque, this patch doesn't require any libc bump? Right. Comments, OKs? ok guenther@
Re: fix seekdir(3)
Hi,
back in November 2013, following an idea by guenther@,
i cooked up another optimization for seekdir(3),
then failed to send out the patch. So here it is.
Philip Guenther wrote on Tue, Nov 05, 2013 at 07:57:43PM -0800:
On Wed, 6 Nov 2013, Ingo Schwarze wrote:
* Worst case:
opendir; telldir
then 100.000 times (seekdir; readdir) at that position
= The entry asked for is the very first entry in the buffer,
which cannot be found, because each dirent only contains
the address of the *following* dirent, so each readdir
triggers getdents anyway, but we still search the whole
buffer each time.
While caching the offset of the start of the buffer is possible, it's
not obvious that that case will happen often enough to be worth it.
Hmm, rewinddir() will _always_ be this worst case. Perhaps rewinddir()
should not call seekdir() and just be the two assignments with lseek(),
skipping the scan of the current buffer.
That wouldn't be very useful because after lseek(2), getdents(2) is
required, which is *much* more expensive than searching the buffer.
It would be assisted by the start-o-buffer cache though.
Exactly. Right now, each rewinddir(3) seeks the whole buffer in vain,
wasting 5 microseconds on that, then proceeds to the extremely expensive
getdents(2) anyway, wasting another 100 microseconds.
The patch to cache the cookie of the first directory entry
in addition to all the other entries that we already cache
is relatively simple, see below.
With this patch, rewinddir(3) changes from always being the worst
case to often being the best case (now always the best case for
directories with less than about 500 entries, or for rewinding
before reading less than about 500 entries), no other case becomes
any worse, and some other, less important edge cases improve, too.
What the patch does is:
* Let opendir(3) save the cookie of the first entry of the directory.
* Let seekdir(3) use that saved cookie to move the internal pointers
to that entry when asked to do so, for example on rewinddir(3).
* Let seekdir(3) update that saved cookie when the buffer is exhausted
and when consequently, an lseek(2) had to be done.
* Let seekdir(3) invalidate the buffer by setting the pointer after
its end, instead of to its beginning.
* Have readdir(3) refrain from invalidating the buffer
when asked to return the first element from it.
This comes with only one price: Adding another off_t to the
opaque struct _dirdesc (= DIR), which already contains another
off_t, two long, two int, and two pointers. That's not much
growth, especially since no sane code will allocate large
numbers of DIRs.
As explained last year, this patch reduced typical execution
times of rewinddir(3) on my notebook for about 100 microseconds
to about 0.05 microseconds.
The /usr/src/regress/lib/libc/telldir/ test suite is still happy,
and so are some other, manual tests i have done.
Am i right that, the type DIR being opaque, this patch doesn't
require any libc bump?
Comments, OKs?
Ingo
Index: opendir.c
===
RCS file: /cvs/src/lib/libc/gen/opendir.c,v
retrieving revision 1.26
diff -u -p -r1.26 opendir.c
--- opendir.c 6 Nov 2013 20:35:25 - 1.26
+++ opendir.c 6 Mar 2014 17:14:16 -
@@ -76,7 +76,7 @@ fdopendir(int fd)
dirp = __fdopendir(fd);
if (dirp != NULL) {
/* Record current offset for immediate telldir() */
- dirp-dd_curpos = lseek(fd, 0, SEEK_CUR);
+ dirp-dd_bufpos = dirp-dd_curpos = lseek(fd, 0, SEEK_CUR);
/*
* POSIX doesn't require fdopendir() to set
@@ -116,6 +116,7 @@ __fdopendir(int fd)
dirp-dd_fd = fd;
dirp-dd_lock = NULL;
dirp-dd_curpos = 0;
+ dirp-dd_bufpos = 0;
return (dirp);
}
Index: readdir.c
===
RCS file: /cvs/src/lib/libc/gen/readdir.c,v
retrieving revision 1.20
diff -u -p -r1.20 readdir.c
--- readdir.c 6 Nov 2013 22:26:14 - 1.20
+++ readdir.c 6 Mar 2014 17:14:16 -
@@ -43,9 +43,8 @@ _readdir_unlocked(DIR *dirp, struct dire
*result = NULL;
for (;;) {
- if (dirp-dd_loc = dirp-dd_size)
+ if (dirp-dd_loc = dirp-dd_size) {
dirp-dd_loc = 0;
- if (dirp-dd_loc == 0) {
dirp-dd_size = getdents(dirp-dd_fd, dirp-dd_buf,
dirp-dd_len);
if (dirp-dd_size == 0)
Index: seekdir.c
===
RCS file: /cvs/src/lib/libc/gen/seekdir.c,v
retrieving revision 1.11
diff -u -p -r1.11 seekdir.c
--- seekdir.c 6 Nov 2013 20:35:25 - 1.11
+++ seekdir.c 6 Mar 2014 17:14:16 -
@@ -36,6 +36,13 @@ seekdir(DIR *dirp, long loc)
*/
_MUTEX_LOCK(dirp-dd_lock);
Re: fix seekdir(3)
Ingo Schwarze wrote on Mon, Nov 04, 2013 at 09:51:41AM +0100:
I will send a minimal one-line patch to just fix the bug and do nothing
else. We should get that one in quickly.
Done.
[...]
Then i will send two cleanup patches to remove useless stuff
and put the code into the right place, not changing any functionality.
Here it is (one patch only).
- put the seekdir() implementation where it belongs
and remove a related lie from a comment
- remove unused headers and an unused prototype
- remove two trivial functions used only once, in the same file
- avoid code duplication
No functional change.
OK?
Ingo
Index: rewinddir.c
===
RCS file: /cvs/src/lib/libc/gen/rewinddir.c,v
retrieving revision 1.10
diff -u -p -r1.10 rewinddir.c
--- rewinddir.c 13 Aug 2013 05:52:12 - 1.10
+++ rewinddir.c 5 Nov 2013 09:54:32 -
@@ -1,5 +1,5 @@
/* $OpenBSD: rewinddir.c,v 1.10 2013/08/13 05:52:12 guenther Exp $ */
-/*-
+/*
* Copyright (c) 1990, 1993
* The Regents of the University of California. All rights reserved.
*
@@ -28,16 +28,12 @@
* SUCH DAMAGE.
*/
-#include sys/types.h
#include dirent.h
-
#include thread_private.h
#include telldir.h
void
rewinddir(DIR *dirp)
{
- _MUTEX_LOCK(dirp-dd_lock);
- __seekdir(dirp, 0);
- _MUTEX_UNLOCK(dirp-dd_lock);
+ seekdir(dirp, 0);
}
Index: seekdir.c
===
RCS file: /cvs/src/lib/libc/gen/seekdir.c,v
retrieving revision 1.9
diff -u -p -r1.9 seekdir.c
--- seekdir.c 5 Jun 2007 18:11:48 - 1.9
+++ seekdir.c 5 Nov 2013 09:54:32 -
@@ -28,19 +28,21 @@
* SUCH DAMAGE.
*/
-#include sys/param.h
#include dirent.h
+#include unistd.h
+
#include thread_private.h
#include telldir.h
/*
* Seek to an entry in a directory.
- * __seekdir is in telldir.c so that it can share opaque data structures.
+ * Only values returned by telldir should be passed to seekdir.
*/
void
seekdir(DIR *dirp, long loc)
{
_MUTEX_LOCK(dirp-dd_lock);
- __seekdir(dirp, loc);
+ dirp-dd_loc = 0;
+ dirp-dd_curpos = lseek(dirp-dd_fd, loc, SEEK_SET);
_MUTEX_UNLOCK(dirp-dd_lock);
}
Index: telldir.c
===
RCS file: /cvs/src/lib/libc/gen/telldir.c,v
retrieving revision 1.16
diff -u -p -r1.16 telldir.c
--- telldir.c 5 Nov 2013 09:36:05 - 1.16
+++ telldir.c 5 Nov 2013 09:54:33 -
@@ -28,45 +28,21 @@
* SUCH DAMAGE.
*/
-#include sys/param.h
-#include sys/queue.h
#include dirent.h
-#include stdlib.h
-#include unistd.h
-
#include thread_private.h
#include telldir.h
-int _readdir_unlocked(DIR *, struct dirent **, int);
-
/*
* return a pointer into a directory
*/
long
-_telldir_unlocked(DIR *dirp)
-{
- return (dirp-dd_curpos);
-}
-
-long
telldir(DIR *dirp)
{
long i;
_MUTEX_LOCK(dirp-dd_lock);
- i = _telldir_unlocked(dirp);
+ i = dirp-dd_curpos;
_MUTEX_UNLOCK(dirp-dd_lock);
return (i);
-}
-
-/*
- * seek to an entry in a directory.
- * Only values returned by telldir should be passed to seekdir.
- */
-void
-__seekdir(DIR *dirp, long loc)
-{
- dirp-dd_loc = 0;
- dirp-dd_curpos = lseek(dirp-dd_fd, loc, SEEK_SET);
}
Index: telldir.h
===
RCS file: /cvs/src/lib/libc/gen/telldir.h,v
retrieving revision 1.6
diff -u -p -r1.6 telldir.h
--- telldir.h 13 Aug 2013 05:52:13 - 1.6
+++ telldir.h 5 Nov 2013 09:54:33 -
@@ -47,7 +47,4 @@ struct _dirdesc {
void*dd_lock; /* mutex to protect struct */
};
-long _telldir_unlocked(DIR *);
-void __seekdir(DIR *, long);
-
#endif
Re: fix seekdir(3)
Ingo Schwarze wrote on Mon, Nov 04, 2013 at 09:51:41AM +0100: Then i will send two cleanup patches to remove useless stuff and put the code into the right place, not changing any functionality. Done committed (thanks to Otto and Todd for checking). Finally, we can work out how to do the optimization. Probably, that will naturally factorize into two steps: (1) Use the information available in the userland buffer to avoid the getdents(2) syscall, *without* assuming monotonicity. That is done by the patch below; i'm asking for OKs. Note that, even though it looks superficially similar to the patch i sent originally, this one does not require monotonicity. To confirm that it is really an optimization, i did some measurements on my notebook: * The typical time required for a seekdir()/readdir() pair when the entry asked for is right at the beginning of the userland buffer is about 0.05 microseconds * The typical time required for searching through the whole userland buffer from its beginning to its end is about 5 microseconds * The typical time required for one getdents() syscall is about 100 microseconds Consequently, we have the following typical edge cases: * Best case: opendir; readdir; telldir then 100.000 times (seekdir; readdir) at that position = The entry asked for is near the beginning of the buffer. For that case, the following patch reduces total execution time from 10.7s to 0.005s (-99.95%). * Maximum searching case: opendir; 500 times readdir; telldir then 100.000 times (seekdir; readdir) at that position = The entry asked for is near the end of the buffer. For that case, the following patch reduces total execution time from 10.7s to 0.5s (-95%). * Worst case: opendir; telldir then 100.000 times (seekdir; readdir) at that position = The entry asked for is the very first entry in the buffer, which cannot be found, because each dirent only contains the address of the *following* dirent, so each readdir triggers getdents anyway, but we still search the whole buffer each time. For that case, the following patch increases total execution time from 10.7s to 11.3s (+5%). So in some cases, very large speedups are possible, for a relatively small cost in the (very unlikely) worst case. (2) Further optimize searching when monotonicity is available. Given the data above, i don't consider further optimization worthwhile any longer, so i consider what i'm sending here to be the final optimization patch. There are two cases where further optimization might in principle be possible based on monotonicity: * Avoid searching through the whole buffer when monotonicity allows to conclude that the entry cannot be anywhere in the buffer. But in that case, getdents() is needed anyway, which is so much more expensive than searching the buffer that the latter is practically irrelevant. * Exploit monotonicity to replace the linear search by a binary search. In the best case - the entries searched for are always near the end of the buffer - that might theoretically reduce execution times by a factor of up to 50. However, that only applies to a very narrow range of problems, specifically directories with several hundred but less than about 500 entries. For larger directories, the buffer cannot hold the whole directory at once, so execution times will be dominated by the required getdents() calls, and the search algorithm becomes irrelevant. So, in the best case, we could save up to 5 microseconds per seekdir()/readdir() pair in directories of 500 entries. That's at most a few milliseconds per iteration through the directory. So, is anybody going to have tens of thousands of directories, each just below 500 entries, and iterate all of them with seekdir()/readdir()? Not likely. I don't think this remote possibility warrants additional complication of the code, and much less introducing a new pathconf(2) value. That said, here is the optimization patch: Yours, Ingo Index: opendir.c === RCS file: /cvs/src/lib/libc/gen/opendir.c,v retrieving revision 1.25 diff -u -p -r1.25 opendir.c --- gen/opendir.c 6 Oct 2013 17:57:11 - 1.25 +++ gen/opendir.c 5 Nov 2013 22:58:40 - @@ -111,6 +111,7 @@ __fdopendir(int fd) return (NULL); } + dirp-dd_size = 0; dirp-dd_loc = 0; dirp-dd_fd = fd; dirp-dd_lock = NULL; Index: seekdir.c === RCS file: /cvs/src/lib/libc/gen/seekdir.c,v retrieving revision 1.10 diff -u -p -r1.10 seekdir.c --- gen/seekdir.c 5 Nov 2013 20:36:51 - 1.10 +++ gen/seekdir.c 5 Nov 2013 22:58:40 - @@ -1,31 +1,18 @@ /* $OpenBSD: seekdir.c,v 1.10 2013/11/05 20:36:51 schwarze Exp
Re: fix seekdir(3)
On Wed, 6 Nov 2013, Ingo Schwarze wrote: Ingo Schwarze wrote on Mon, Nov 04, 2013 at 09:51:41AM +0100: Finally, we can work out how to do the optimization. Probably, that will naturally factorize into two steps: (1) Use the information available in the userland buffer to avoid the getdents(2) syscall, *without* assuming monotonicity. That is done by the patch below; i'm asking for OKs. Note that, even though it looks superficially similar to the patch i sent originally, this one does not require monotonicity. To confirm that it is really an optimization, i did some measurements on my notebook: ... * Worst case: opendir; telldir then 100.000 times (seekdir; readdir) at that position = The entry asked for is the very first entry in the buffer, which cannot be found, because each dirent only contains the address of the *following* dirent, so each readdir triggers getdents anyway, but we still search the whole buffer each time. While caching the offset of the start of the buffer is possible, it's not obvious that that case will happen often enough to be worth it. Hmm, rewinddir() will _always_ be this worst case. Perhaps rewinddir() should not call seekdir() and just be the two assignments with lseek(), skipping the scan of the current buffer. It would be assisted by the start-o-buffer cache though. (2) Further optimize searching when monotonicity is available. Given the data above, i don't consider further optimization worthwhile any longer, so i consider what i'm sending here to be the final optimization patch. There are two cases where further optimization might in principle be possible based on monotonicity: * Avoid searching through the whole buffer when monotonicity allows to conclude that the entry cannot be anywhere in the buffer. But in that case, getdents() is needed anyway, which is so much more expensive than searching the buffer that the latter is practically irrelevant. Yeah, doesn't seem like much a win. * Exploit monotonicity to replace the linear search by a binary search. Doing a binary search would be quite tricky given the variable-size of the entries. Not worth it. That said, here is the optimization patch: Looks good. Nice analysis. ok guenther@
Re: fix seekdir(3)
On Sat, Nov 2, 2013 at 5:48 PM, Ingo Schwarze schwa...@usta.de wrote:
Here is an updated patch which now works correctly with Otto's
regression test, with the new test i just committed, and with
the test from the Perl test suite Andrew pointed out, even with
threads enabled. It also survived quite some manual testing.
...
Comments? Tests? OKs?
...
void
seekdir(DIR *dirp, long loc)
{
+ struct dirent *dp;
+
+ /*
+* First check whether the directory entry to seek for
+* is still buffered in the directory structure in memory.
+*/
+
_MUTEX_LOCK(dirp-dd_lock);
- __seekdir(dirp, loc);
+ dp = (struct dirent *)dirp-dd_buf;
+ if (dirp-dd_size 0 dp-d_off = loc) {
+ for (dirp-dd_loc = 0;
+dirp-dd_loc dirp-dd_size;
+dirp-dd_loc += dp-d_reclen) {
+ dp = (struct dirent *)(dirp-dd_buf + dirp-dd_loc);
+ if (dp-d_off loc)
+ continue;
This diff assumes that the d_off values for directory entries are
monotonically increasing as you advance through the directory. That
is not guaranteed and is false for our implementation for some
filesystems. The d_off values for NFS come straight from the NFS
server, and the NFS RFCs place no requirement on the cookies returned
by the server other than that the zero cookie indicates the first
entry. (An OpenBSD NFS server returns cookies from the underlying
filesystem, so this will be difficult to see on a normal system.)
The current tmpfs code will also cause problems for this diff: the
d_off values for tmpfs are practically random, being derived from
kernel pointers returned by malloc(9). That will need to change, as
kernel pointer values shouldn't be exposed to userland like that, but
it should serve as a way to confirm that this behavior causes problems
for the optimized seekdir().
One possibility is for the kernel to tell userland whether d_off
values are sure to be monotonically increasing for the opened
directory and then have userland only use the opimization when that's
the case. We could add a pathconf/fpathconf name to get that
information: fpathconf(fd, _PC_DIRECTORY_MONOTONIC_OFFSETS) anyone?
Philip Guenther
Re: fix seekdir(3)
Hi Philip,
thanks for looking at this and for your insight. The number of aspects
to keep in mind about this patch is growing, we have to disentangle.
I will send a minimal one-line patch to just fix the bug and do nothing
else. We should get that one in quickly.
That would also be a candidate for -stable, i think.
I hope to come round to that tonight.
Then i will send two cleanup patches to remove useless stuff
and put the code into the right place, not changing any functionality.
That will make the cleanup easier to review.
Finally, we can work out how to do the optimization.
Probably, that will naturally factorize into two steps:
(1) Use the information available in the userland buffer
to avoid the getdents(2) syscall, *without* assuming
monotonicity.
(2) Further optimize searching when monotonicity is available.
Yours,
Ingo
Philip Guenther wrote on Mon, Nov 04, 2013 at 12:26:34AM -0800:
On Sat, Nov 2, 2013 at 5:48 PM, Ingo Schwarze schwa...@usta.de wrote:
Here is an updated patch which now works correctly with Otto's
regression test, with the new test i just committed, and with
the test from the Perl test suite Andrew pointed out, even with
threads enabled. It also survived quite some manual testing.
...
Comments? Tests? OKs?
...
void
seekdir(DIR *dirp, long loc)
{
+ struct dirent *dp;
+
+ /*
+* First check whether the directory entry to seek for
+* is still buffered in the directory structure in memory.
+*/
+
_MUTEX_LOCK(dirp-dd_lock);
- __seekdir(dirp, loc);
+ dp = (struct dirent *)dirp-dd_buf;
+ if (dirp-dd_size 0 dp-d_off = loc) {
+ for (dirp-dd_loc = 0;
+dirp-dd_loc dirp-dd_size;
+dirp-dd_loc += dp-d_reclen) {
+ dp = (struct dirent *)(dirp-dd_buf + dirp-dd_loc);
+ if (dp-d_off loc)
+ continue;
This diff assumes that the d_off values for directory entries are
monotonically increasing as you advance through the directory. That
is not guaranteed and is false for our implementation for some
filesystems. The d_off values for NFS come straight from the NFS
server, and the NFS RFCs place no requirement on the cookies returned
by the server other than that the zero cookie indicates the first
entry. (An OpenBSD NFS server returns cookies from the underlying
filesystem, so this will be difficult to see on a normal system.)
The current tmpfs code will also cause problems for this diff: the
d_off values for tmpfs are practically random, being derived from
kernel pointers returned by malloc(9). That will need to change, as
kernel pointer values shouldn't be exposed to userland like that, but
it should serve as a way to confirm that this behavior causes problems
for the optimized seekdir().
One possibility is for the kernel to tell userland whether d_off
values are sure to be monotonically increasing for the opened
directory and then have userland only use the opimization when that's
the case. We could add a pathconf/fpathconf name to get that
information: fpathconf(fd, _PC_DIRECTORY_MONOTONIC_OFFSETS) anyone?
Re: fix seekdir(3)
Ingo Schwarze wrote on Mon, Nov 04, 2013 at 09:51:41AM +0100:
I will send a minimal one-line patch to just fix the bug
and do nothing else. We should get that one in quickly.
That would also be a candidate for -stable, i think.
I hope to come round to that tonight.
Here it is.
This fixes both our own regression tests (Ottos and mine)
and the perl-5.18 op/threads-dirh.t test.
This is not particularly efficient, forcing getdents(2) for each
readdir(3) following seekdir(3), but at least it produces correct
results, for a start.
OK?
Ingo
Index: telldir.c
===
RCS file: /cvs/src/lib/libc/gen/telldir.c,v
retrieving revision 1.15
diff -u -p -r1.15 telldir.c
--- telldir.c 16 Aug 2013 05:27:39 - 1.15
+++ telldir.c 4 Nov 2013 21:11:55 -
@@ -67,5 +67,6 @@ telldir(DIR *dirp)
void
__seekdir(DIR *dirp, long loc)
{
+ dirp-dd_loc = 0;
dirp-dd_curpos = lseek(dirp-dd_fd, loc, SEEK_SET);
}
Re: fix seekdir(3)
On Monday, November 4, 2013, Ingo Schwarze wrote: This fixes both our own regression tests (Ottos and mine) and the perl-5.18 op/threads-dirh.t test. This is not particularly efficient, forcing getdents(2) for each readdir(3) following seekdir(3), but at least it produces correct results, for a start. OK? Yep, ok guenther@
Re: fix seekdir(3)
Hi,
after some more testing and proofreading, i found some issues
with my libc/gen patch; please delete what i sent yesterday.
Here is an updated patch which now works correctly with Otto's
regression test, with the new test i just committed, and with
the test from the Perl test suite Andrew pointed out, even with
threads enabled. It also survived quite some manual testing.
Changes with respect to yesterday:
* Unfortunately, dirent.d_off is the offset of the *next*
entry with respect to the beginning of the directory,
not of the *current* entry. So, we simply don't know
where the first entry in dd_buf is located in the directory,
so we cannot seek back to it without calling getdents() again.
Thus, we can as well continue using dd_loc = 0 as the flag
to invalidate dd_buf, no use changing it to -1.
* Due to the same misunderstanding of d_off, i introduced an
off-by-one error, sometimes backing up one entry too far.
Ouch.
* Initialize dd_size in opendir() and test it in seekdir()
to make sure that we don't access dd_buf uninitialized.
Comments? Tests? OKs?
Ingo
P.S.
Oh, and as requested by Theo, i'm now fixing the bug 90 years
earlier than i originally intended.
Index: opendir.c
===
RCS file: /cvs/src/lib/libc/gen/opendir.c,v
retrieving revision 1.25
diff -u -p -r1.25 opendir.c
--- opendir.c 6 Oct 2013 17:57:11 - 1.25
+++ opendir.c 3 Nov 2013 00:23:09 -
@@ -111,6 +111,7 @@ __fdopendir(int fd)
return (NULL);
}
+ dirp-dd_size = 0;
dirp-dd_loc = 0;
dirp-dd_fd = fd;
dirp-dd_lock = NULL;
Index: rewinddir.c
===
RCS file: /cvs/src/lib/libc/gen/rewinddir.c,v
retrieving revision 1.10
diff -u -p -r1.10 rewinddir.c
--- rewinddir.c 13 Aug 2013 05:52:12 - 1.10
+++ rewinddir.c 3 Nov 2013 00:23:09 -
@@ -1,5 +1,5 @@
/* $OpenBSD: rewinddir.c,v 1.10 2013/08/13 05:52:12 guenther Exp $ */
-/*-
+/*
* Copyright (c) 1990, 1993
* The Regents of the University of California. All rights reserved.
*
@@ -28,16 +28,12 @@
* SUCH DAMAGE.
*/
-#include sys/types.h
#include dirent.h
-
#include thread_private.h
#include telldir.h
void
rewinddir(DIR *dirp)
{
- _MUTEX_LOCK(dirp-dd_lock);
- __seekdir(dirp, 0);
- _MUTEX_UNLOCK(dirp-dd_lock);
+ seekdir(dirp, 0);
}
Index: seekdir.c
===
RCS file: /cvs/src/lib/libc/gen/seekdir.c,v
retrieving revision 1.9
diff -u -p -r1.9 seekdir.c
--- seekdir.c 5 Jun 2007 18:11:48 - 1.9
+++ seekdir.c 3 Nov 2013 00:23:09 -
@@ -1,46 +1,71 @@
/* $OpenBSD: seekdir.c,v 1.9 2007/06/05 18:11:48 kurt Exp $ */
/*
- * Copyright (c) 1983, 1993
- * The Regents of the University of California. All rights reserved.
+ * Copyright (c) 2013 Ingo Schwarze schwa...@openbsd.org
*
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *notice, this list of conditions and the following disclaimer in the
- *documentation and/or other materials provided with the distribution.
- * 3. Neither the name of the University nor the names of its contributors
- *may be used to endorse or promote products derived from this software
- *without specific prior written permission.
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
*
- * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
- * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
+ * THE SOFTWARE IS PROVIDED AS IS AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR
Re: fix seekdir(3), was: update perl Module::Build
On Sat, Nov 02, 2013 at 05:27:38AM +0100, Ingo Schwarze wrote:
Andrew Fresh wrote on Wed, Oct 30, 2013 at 01:50:56PM -0700:
Post vBSDcon I have most existing patches working with 5.18.1, the only
failing so far is that with threads enabled
If i understand correctly, we do not currently have threads enabled
in our -current base perl 5.16, so that is not a regression.
You are correct, I had threads enabled in the github repo and forgot
that it hadn't actually made it in-tree.
So, if that is all that is broken, you are basically claiming
that perl 5.18.1 is ready for commit? ;-)
It seems close, I'd like to see some test reports on other
architectures, but since the lack of threads isn't actually a problem, I
will set up some of the other arch's that I have and see about testing
it on those.
I also do want to actually spend more time looking and trying to
understand the diff between 5.16.3 and 5.18.1 to make sure there wasn't
anything introduced that I had questions about. Perhaps helping unload
a moving truck this weekend will take less time than I expect.
t/op/threads-dirh.t fails.
(mv patches/{APPLIES,GOOD}/use_threads.patch)
This test appears to point to a failure that needs fixing but I am not
skilled enough to know how.
That problem is neither related to threads nor to 5.18 nor even to
Perl at all! The problem is that the seekdir(3) in our C library
is badly broken: The readdir(3) function saves dirents in a buffer,
so the lseek(2) done by seekdir(3) only takes effect after that
buffer becomes exhausted. Otto's nice regression test doesn't catch
that breakage because it only seeks far, far away, never in the
neighbourhood.
This is amazing, thank you much, I will have to drink some extra coffee
and try to understand that as well.
l8rZ,
--
andrew - http://afresh1.com
The 3 great virtues of a programmer: Laziness, Impatience, and Hubris.
--Larry Wall
