Re: seekdir/readdir patch .. Call for Review.
On 03.05.2015 16:01, Julian Elischer wrote: Before making single-purpose changes to the libc readdir and seekdir code, or to the kernel code, it would be useful to state exact behaviour of the dirent machinery we want to see. No, 'make samba works in my situation' does not sound good enough. Well samba is a MAJOR application for FreeBSD. there are many people who combine the two, (e.g. FreeNAS, which suffers from this problem) so taking it from The Samba community does not recommend running on FreeBSD due to problems with seekdir to Samba works correctly on FreeBSD is important.. Samba's behaviour here is governed by Windows expectiations. Please look at https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=198819 too. Perhaps it is related or may be not (sorry currently I am not able to inspect the code). -- http://ache.vniz.net/ ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Jenkins build is back to normal : FreeBSD_HEAD_i386 #76
See https://jenkins.freebsd.org/job/FreeBSD_HEAD_i386/76/changes ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: seekdir/readdir patch .. Call for Review.
On 5/3/15 10:33 PM, Jilles Tjoelker wrote: On Fri, May 01, 2015 at 07:17:42PM +0300, Konstantin Belousov wrote: On Fri, May 01, 2015 at 03:04:51PM +0800, Julian Elischer wrote: if you are interested in readdir(3), seekdir(3) and telldir(3) then you should look at https://reviews.freebsd.org/D2410 this patches around a problem in seekdir() that breaks Samba. Seekdir(3) will not work as expected when files prior to the point of interest in directory have been deleted since the directory was opened. Windows clients using Samba cause both these things to happen, causing the next readdir(3) after the bad seekdir(3) to skip some entries and return the wrong file. Samba only needs to step back a single directory entry in the case where it reads an entry and then discovers it can't fit it into the buffer it is sending to the windows client. It turns out we can reliably cater to Samba's requirement because the last returned element is always still in memory, so with a little care, we can set our filepointer back to it safely. (once) seekdir and readdir (and telldir()) need a complete rewrite along with getdirentries() but that is more than a small edit like this. Can you explain your expectations from the whole readdir() vs. parallel directory modifications interaction ? From what I understood so far, there is unlocked modification of the container and parallel iterator over the same container. IMO, in such situation, whatever tweaks you apply to the iterator, it is still cannot be made reliable. Before making single-purpose changes to the libc readdir and seekdir code, or to the kernel code, it would be useful to state exact behaviour of the dirent machinery we want to see. No, 'make samba works in my situation' does not sound good enough. Consider the subsequence of entries that existed at opendir() time and were not removed until now. This subsequence is clearly defined and does not have concurrency problems. The order of this subsequence must remain unchanged and seekdir() must be correct with respect to this subsequence. Additionally, two other kinds of entries may be returned. New entries may be inserted anywhere in between the entries of the subsequence, and removed entries may be returned as if they were still part of the subsequence (so that not every readdir() needs a system call). A simple implementation for UFS-style directories is to store the offset in the directory (all bits of it, not masking off the lower 9 bits). This needs d_off or similar in struct dirent. The kernel getdirentries() then needs a similar loop as the old libc seekdir() to go from the start of the 512-byte directory block to the desired entry (since an entry may not exist at the stored offset within the directory block). This means that a UFS-style directory cannot be compacted (existing entries moved from higher to lower offsets to fill holes) while it is open for reading. An NFS exported directory is always open for reading. This also means that duplicate entries can only be returned if that particular filename was deleted and created again. Without kernel support, it is hard to get telldir/seekdir completely reliable. The current libc implementation is wrong since the holes within the block just disappear and change the offsets of the following entries; the kernel cannot fix this using entries with d_fileno = 0 since it cannot know, in the general case, how long the deleted entry was in the filesystem-independent dirent format. My previous idea of storing one d_fileno during telldir() is wrong since it will fail if that entry is deleted. If you do not care about memory usage (which probably is already excessive with the current libc implementation), you could store at telldir() time the offset of the current block returned by getdirentries() and the d_fileno of all entries already returned in the current block. The D2410 patch can conceptually work for what Samba needs, stepping back one directory entry. I will comment on it. thanks your comment is correct, but I don't think it really matters because I'm only claiming to fix a really small set of possible usages.. I might add a sentence in the seekdir man page specifying what does and doesn't work. ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: seekdir/readdir patch .. Call for Review.
On 5/2/15 12:17 AM, Konstantin Belousov wrote: On Fri, May 01, 2015 at 03:04:51PM +0800, Julian Elischer wrote: if you are interested in readdir(3), seekdir(3) and telldir(3) then you should look at https://reviews.freebsd.org/D2410 this patches around a problem in seekdir() that breaks Samba. Seekdir(3) will not work as expected when files prior to the point of interest in directory have been deleted since the directory was opened. Windows clients using Samba cause both these things to happen, causing the next readdir(3) after the bad seekdir(3) to skip some entries and return the wrong file. Samba only needs to step back a single directory entry in the case where it reads an entry and then discovers it can't fit it into the buffer it is sending to the windows client. It turns out we can reliably cater to Samba's requirement because the last returned element is always still in memory, so with a little care, we can set our filepointer back to it safely. (once) seekdir and readdir (and telldir()) need a complete rewrite along with getdirentries() but that is more than a small edit like this. Can you explain your expectations from the whole readdir() vs. parallel directory modifications interaction ? From what I understood so far, there is unlocked modification of the container and parallel iterator over the same container. IMO, in such situation, whatever tweaks you apply to the iterator, it is still cannot be made reliable. Before making single-purpose changes to the libc readdir and seekdir code, or to the kernel code, it would be useful to state exact behaviour of the dirent machinery we want to see. No, 'make samba works in my situation' does not sound good enough. Well samba is a MAJOR application for FreeBSD. there are many people who combine the two, (e.g. FreeNAS, which suffers from this problem) so taking it from The Samba community does not recommend running on FreeBSD due to problems with seekdir to Samba works correctly on FreeBSD is important.. Samba's behaviour here is governed by Windows expectiations. I am specifically NOT changing kernel code.. though in discussion with jhb and others, it becomes clear that this will have to happen some time in the future. There is no way to make seekdir 100% reliable without going to exposing cookies through getdirentries(), and even then it relies on the file systems doing the right thing with the cookies. Don't let Perfect be the enemy of 'better'. I think hte two changes here make seekdir much more reliable. not only does it make the 'back up one entry' case 100% reliable, but it also makes any backwards seek within the current buffer also work correctly. The expectation is that if you read an entry and then immediately set the pointer back by one so that you can read it again, that hte next readdir() will once again return the same entry regardless of whether any deletes have happenned between the seekdir and the readdir() In other words if we 'pre-read' an item to find its size, and then read it again.. we get the same item. the change is small, makes it more reliable and helps a number of FreeBSD users (e.g. FreeNAS). I don't see why it would be objectionable to anyone. unless you are relying an samba failing to delete random files when you try delete a subdirectory. ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
mergemaster failing with read-only /usr/src
Hi, I'm trying to update this system: FreeBSD pomona 11.0-CURRENT FreeBSD 11.0-CURRENT #0: Mon Apr 13 03:48:04 CEST 2015 wolfgang@pomona:/usr/obj/usr/src/sys/UBQTERL mips Source for that was probably from about April 11th. I sucessfully built world and kernel, ran mergemaster -p and make installworld on rev 282299 but then mergemaster fails with: # mergemaster -iFU *** Creating the temporary root environment in /var/tmp/temproot *** /var/tmp/temproot ready for use *** Creating and populating directory structure in /var/tmp/temproot /bin/sh: cannot create routing_test.tmp: Read-only file system *** FATAL ERROR: Cannot 'cd' to /usr/src and install files to the temproot environment Filesystems are mounted like this: # mount /dev/da0s2a on / (ufs, local, noatime) devfs on /dev (devfs, local, multilabel) /dev/da0s1 on /boot (msdosfs, local) vulcan.lyx:/usr/src11 on /usr/src (nfs, read-only) vulcan.lyx:/var/obj/11/mips64 on /usr/obj (nfs) This used to work before. Any ideas, any further info I could provide? Wolfgang ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: mergemaster failing with read-only /usr/src
* Jilles Tjoelker jil...@stack.nl [150503 14:53]: On Sun, May 03, 2015 at 02:03:49PM +0200, Wolfgang Zenker wrote: I'm trying to update this system: FreeBSD pomona 11.0-CURRENT FreeBSD 11.0-CURRENT #0: Mon Apr 13 03:48:04 CEST 2015 wolfgang@pomona:/usr/obj/usr/src/sys/UBQTERL mips Source for that was probably from about April 11th. I sucessfully built world and kernel, ran mergemaster -p and make installworld on rev 282299 but then mergemaster fails with: # mergemaster -iFU *** Creating the temporary root environment in /var/tmp/temproot *** /var/tmp/temproot ready for use *** Creating and populating directory structure in /var/tmp/temproot /bin/sh: cannot create routing_test.tmp: Read-only file system *** FATAL ERROR: Cannot 'cd' to /usr/src and install files to the temproot environment Filesystems are mounted like this: # mount /dev/da0s2a on / (ufs, local, noatime) devfs on /dev (devfs, local, multilabel) /dev/da0s1 on /boot (msdosfs, local) vulcan.lyx:/usr/src11 on /usr/src (nfs, read-only) vulcan.lyx:/var/obj/11/mips64 on /usr/obj (nfs) This used to work before. Any ideas, any further info I could provide? This broke after a test was added for etc/rc.d/. Without special code, this causes these tests to be built and installed as part of mergemaster/etcmerge, like other parts of etc. As a workaround you can do: echo make -C etc obj all | make buildenv on the build machine after make buildworld. Then mergemaster will work, even with a read-only /usr/obj. Well, I do build on that machine directly, and /usr/obj is mounted r/w, only /usr/src is a read-only mount. Trying the workaround on the machine istself does not help, unfortunately: while the make buildenv does work without a problem, mergemaster still fails in the same way. Wolfgang ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: mergemaster failing with read-only /usr/src
On Sun, May 03, 2015 at 02:03:49PM +0200, Wolfgang Zenker wrote: I'm trying to update this system: FreeBSD pomona 11.0-CURRENT FreeBSD 11.0-CURRENT #0: Mon Apr 13 03:48:04 CEST 2015 wolfgang@pomona:/usr/obj/usr/src/sys/UBQTERL mips Source for that was probably from about April 11th. I sucessfully built world and kernel, ran mergemaster -p and make installworld on rev 282299 but then mergemaster fails with: # mergemaster -iFU *** Creating the temporary root environment in /var/tmp/temproot *** /var/tmp/temproot ready for use *** Creating and populating directory structure in /var/tmp/temproot /bin/sh: cannot create routing_test.tmp: Read-only file system *** FATAL ERROR: Cannot 'cd' to /usr/src and install files to the temproot environment Filesystems are mounted like this: # mount /dev/da0s2a on / (ufs, local, noatime) devfs on /dev (devfs, local, multilabel) /dev/da0s1 on /boot (msdosfs, local) vulcan.lyx:/usr/src11 on /usr/src (nfs, read-only) vulcan.lyx:/var/obj/11/mips64 on /usr/obj (nfs) This used to work before. Any ideas, any further info I could provide? This broke after a test was added for etc/rc.d/. Without special code, this causes these tests to be built and installed as part of mergemaster/etcmerge, like other parts of etc. As a workaround you can do: echo make -C etc obj all | make buildenv on the build machine after make buildworld. Then mergemaster will work, even with a read-only /usr/obj. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: seekdir/readdir patch .. Call for Review.
On Fri, May 01, 2015 at 07:17:42PM +0300, Konstantin Belousov wrote: On Fri, May 01, 2015 at 03:04:51PM +0800, Julian Elischer wrote: if you are interested in readdir(3), seekdir(3) and telldir(3) then you should look at https://reviews.freebsd.org/D2410 this patches around a problem in seekdir() that breaks Samba. Seekdir(3) will not work as expected when files prior to the point of interest in directory have been deleted since the directory was opened. Windows clients using Samba cause both these things to happen, causing the next readdir(3) after the bad seekdir(3) to skip some entries and return the wrong file. Samba only needs to step back a single directory entry in the case where it reads an entry and then discovers it can't fit it into the buffer it is sending to the windows client. It turns out we can reliably cater to Samba's requirement because the last returned element is always still in memory, so with a little care, we can set our filepointer back to it safely. (once) seekdir and readdir (and telldir()) need a complete rewrite along with getdirentries() but that is more than a small edit like this. Can you explain your expectations from the whole readdir() vs. parallel directory modifications interaction ? From what I understood so far, there is unlocked modification of the container and parallel iterator over the same container. IMO, in such situation, whatever tweaks you apply to the iterator, it is still cannot be made reliable. Before making single-purpose changes to the libc readdir and seekdir code, or to the kernel code, it would be useful to state exact behaviour of the dirent machinery we want to see. No, 'make samba works in my situation' does not sound good enough. Consider the subsequence of entries that existed at opendir() time and were not removed until now. This subsequence is clearly defined and does not have concurrency problems. The order of this subsequence must remain unchanged and seekdir() must be correct with respect to this subsequence. Additionally, two other kinds of entries may be returned. New entries may be inserted anywhere in between the entries of the subsequence, and removed entries may be returned as if they were still part of the subsequence (so that not every readdir() needs a system call). A simple implementation for UFS-style directories is to store the offset in the directory (all bits of it, not masking off the lower 9 bits). This needs d_off or similar in struct dirent. The kernel getdirentries() then needs a similar loop as the old libc seekdir() to go from the start of the 512-byte directory block to the desired entry (since an entry may not exist at the stored offset within the directory block). This means that a UFS-style directory cannot be compacted (existing entries moved from higher to lower offsets to fill holes) while it is open for reading. An NFS exported directory is always open for reading. This also means that duplicate entries can only be returned if that particular filename was deleted and created again. Without kernel support, it is hard to get telldir/seekdir completely reliable. The current libc implementation is wrong since the holes within the block just disappear and change the offsets of the following entries; the kernel cannot fix this using entries with d_fileno = 0 since it cannot know, in the general case, how long the deleted entry was in the filesystem-independent dirent format. My previous idea of storing one d_fileno during telldir() is wrong since it will fail if that entry is deleted. If you do not care about memory usage (which probably is already excessive with the current libc implementation), you could store at telldir() time the offset of the current block returned by getdirentries() and the d_fileno of all entries already returned in the current block. The D2410 patch can conceptually work for what Samba needs, stepping back one directory entry. I will comment on it. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: mergemaster failing with read-only /usr/src
On May 3, 2015, at 8:55, Wolfgang Zenker wolfg...@lyxys.ka.sub.org wrote: * Jilles Tjoelker jil...@stack.nl [150503 14:53]: On Sun, May 03, 2015 at 02:03:49PM +0200, Wolfgang Zenker wrote: I'm trying to update this system: FreeBSD pomona 11.0-CURRENT FreeBSD 11.0-CURRENT #0: Mon Apr 13 03:48:04 CEST 2015 wolfgang@pomona:/usr/obj/usr/src/sys/UBQTERL mips Source for that was probably from about April 11th. I sucessfully built world and kernel, ran mergemaster -p and make installworld on rev 282299 but then mergemaster fails with: # mergemaster -iFU *** Creating the temporary root environment in /var/tmp/temproot *** /var/tmp/temproot ready for use *** Creating and populating directory structure in /var/tmp/temproot /bin/sh: cannot create routing_test.tmp: Read-only file system *** FATAL ERROR: Cannot 'cd' to /usr/src and install files to the temproot environment Filesystems are mounted like this: # mount /dev/da0s2a on / (ufs, local, noatime) devfs on /dev (devfs, local, multilabel) /dev/da0s1 on /boot (msdosfs, local) vulcan.lyx:/usr/src11 on /usr/src (nfs, read-only) vulcan.lyx:/var/obj/11/mips64 on /usr/obj (nfs) This used to work before. Any ideas, any further info I could provide? This broke after a test was added for etc/rc.d/. Without special code, this causes these tests to be built and installed as part of mergemaster/etcmerge, like other parts of etc. As a workaround you can do: echo make -C etc obj all | make buildenv on the build machine after make buildworld. Then mergemaster will work, even with a read-only /usr/obj. Well, I do build on that machine directly, and /usr/obj is mounted r/w, only /usr/src is a read-only mount. Trying the workaround on the machine istself does not help, unfortunately: while the make buildenv does work without a problem, mergemaster still fails in the same way. I was going to move it to etc/tests soon since it wasn’t really testing /etc/rc.d/, but it makes more sense (with the issue above), just to create .../tests/etc, and move things there. I wish etc/ wasn’t such a special butterfly... signature.asc Description: Message signed with OpenPGP using GPGMail
