Re: CVS commit: src
On Thu, May 07, 2020 at 10:52:26PM +0200, Yorick Hardy wrote: > Thanks! It took a while for this to sink in ... > > So I think the test should only be built if MKCOMPAT=yes ? > (It does fail to build for me, because I usually have MKCOMPAT=no). Yes, either the Makefile needs a conditional, or the SUBDIR+= in the makefile above. Martin
Re: CVS commit: src
Dear Martin,
On 2020-05-07, Martin Husemann wrote:
> On Thu, May 07, 2020 at 12:31:12AM +0200, Yorick Hardy wrote:
> > I think this test depends on MKCOMPAT=yes. Does the attached the patch
> > below look reasonable?
> [..]
> > -.if ${MACHINE} == "amd64"
> > +.if ${MACHINE} == "amd64" && ${MKCOMPATTESTS} == "yes"
>
> MKCOMPATTESTS is something slightly different. It is used to build e.g. the
> "native" i386 tests (as 32bit binaries) when building an amd64 distribution.
>
> This is usually set to "no".
>
> MKCOMPAT=yes means to build i386 libraries, so you can build 32bit binaries
> on amd64 with -m32. It is usually set to "yes".
>
> Martin
Thanks! It took a while for this to sink in ...
So I think the test should only be built if MKCOMPAT=yes ?
(It does fail to build for me, because I usually have MKCOMPAT=no).
--
Kind regards,
Yorick Hardy
Re: CVS commit: src/external/cddl/osnet/dist/uts/common/fs/zfs
> On May 7, 2020, at 5:47 PM, Taylor R Campbell > wrote: > >> Module Name:src >> Committed By: hannken >> Date: Thu May 7 09:12:03 UTC 2020 >> >> Modified Files: >>src/external/cddl/osnet/dist/uts/common/fs/zfs: zfs_vnops.c >> >> Log Message: >> Revert Rev. 1.63 and add a comment why we have to zil_commit() here: >> >> Operation zfs_znode.c::zfs_zget_cleaner() depends on this >> zil_commit() as a barrier to guarantee the znode cannot >> get freed before its log entries are resolved. > > We must be doing something wrong. > > The only times we should ever call zil_commit are when someone called > fsync or the file system is mounted sync=always. Calling zil_commit > whenever we delete a file absolutely wrecks performance and shouldn't > be needed for on-disk correctness in normal zfs semantics unless I > terribly misunderstand something, so we must be doing something wrong > with the in-memory state if we seem to need this. The problem is the way we get data in zfs_get_data(). Other OS use zfs_zget() to obtain a referenced vnode/znode and use it to retrieve tha data. For us this results in deadlocks either as locking-against-self if called during vcache_reclaim() or more difficult deadlocks as another operation needs to proceed and we currently have txg stopped from syncing. Chuq resolved it with zfs_zget_cleaner() that doesn't reference the vnode/znode and works for in-memory znodes only so we have to guarantee it doesn't get called after VOP_RECLAIM(). > Do you have a test case that can trigger the problem? Under load I get use-after-free of znodes in zfs_get_data() or entirely miss znodes here. See the other commit asserting zfs_zget_cleaner() never fails. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany) signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/external/cddl/osnet/dist/uts/common/fs/zfs
> Module Name:src > Committed By: hannken > Date: Thu May 7 09:12:03 UTC 2020 > > Modified Files: > src/external/cddl/osnet/dist/uts/common/fs/zfs: zfs_vnops.c > > Log Message: > Revert Rev. 1.63 and add a comment why we have to zil_commit() here: > > Operation zfs_znode.c::zfs_zget_cleaner() depends on this > zil_commit() as a barrier to guarantee the znode cannot > get freed before its log entries are resolved. We must be doing something wrong. The only times we should ever call zil_commit are when someone called fsync or the file system is mounted sync=always. Calling zil_commit whenever we delete a file absolutely wrecks performance and shouldn't be needed for on-disk correctness in normal zfs semantics unless I terribly misunderstand something, so we must be doing something wrong with the in-memory state if we seem to need this. Do you have a test case that can trigger the problem?
Re: CVS commit: src/sys/kern
On 07.05.2020 07:46, Michael van Elst wrote: > On Wed, May 06, 2020 at 12:39:21PM +0200, Kamil Rytarowski wrote: > > Hi Kamil, > >> If I revert the pipe(2) changes on top of NetBSD-current, the test is no >> longer racy again. > > I tried 9.99.60 with and without my bugfix. The test always failed > after some time with exactly that error. > > If the change really had an effect, there would be a use-after-free bug > somewhere else. > > > Greetings, > Thanks, I will investigating further. signature.asc Description: OpenPGP digital signature
