Re: CVS commit: src/distrib/sets/lists/comp
On Feb 24, 2014, at 10:14 PM, Joerg Sonnenberger wrote: > Module Name: src > Committed By: joerg > Date: Mon Feb 24 21:14:13 UTC 2014 > > Modified Files: > src/distrib/sets/lists/comp: md.sparc64 > > Log Message: > Fix compat entries. > > > To generate a diff of this commit: > cvs rdiff -u -r1.179 -r1.180 src/distrib/sets/lists/comp/md.sparc64 This breaks sparc64 release build: === 2 extra files in DESTDIR = Files in DESTDIR but missing from flist. File is obsolete or flist is out of date ? -- ./usr/include/g++/bits/sparc ./usr/include/g++/bits/sparc64 = end of 2 extra files === Reverting this change makes sparc64 build again. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/distrib/sets/lists/comp
On Thu, Feb 27, 2014 at 10:07:36AM +0100, J. Hannken-Illjes wrote: > ./usr/include/g++/bits/sparc > ./usr/include/g++/bits/sparc64 I asked Jörg to try (his MKCOMPAT=no clang build) with ./usr/include/g++/bits/sparcgcc,compat ./usr/include/g++/bits/sparc64 gcc (the old state was obviosly incorrect, but so is the current one). This is still incorrect for gcc 4.1, but I bet noone will notice. Martin
Re: CVS commit: src
"Juergen Hannken-Illjes" wrote: > Module Name: src > Committed By: hannken > Date: Thu Feb 27 16:51:39 UTC 2014 > > <...> > > Log Message: > The current implementation of vn_lock() is racy. Modification of > the vnode operations vector for active vnodes is unsafe because it > is not known whether deadfs or the original file system will be > called. > > <...> > I have not had time to follow your VFS changes, but can you explain why did you remove VOP_LOCK/VOP_UNLOCK in tmpfs_reclaim()? It was added to prevent from the racy access of tn_links. -- Mindaugas
Re: CVS commit: src
On Feb 27, 2014, at 6:06 PM, Mindaugas Rasiukevicius wrote: > "Juergen Hannken-Illjes" wrote: >> Module Name: src >> Committed By:hannken >> Date:Thu Feb 27 16:51:39 UTC 2014 >> >> <...> >> >> Log Message: >> The current implementation of vn_lock() is racy. Modification of >> the vnode operations vector for active vnodes is unsafe because it >> is not known whether deadfs or the original file system will be >> called. >> >> <...> >> > > I have not had time to follow your VFS changes, but can you explain why did > you remove VOP_LOCK/VOP_UNLOCK in tmpfs_reclaim()? It was added to prevent > from the racy access of tn_links. Hopefully a vnode lock is needed to access tn_links -- otherwise the VOP_LOCK/UNLOCK in tmpfs_reclaim() didn't protect it ... As this commit prevents other threads from obtaining a lock while VOP_RECLAIM() is running the protection should be the same as before. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src
"J. Hannken-Illjes" wrote: > > I have not had time to follow your VFS changes, but can you explain why > > did you remove VOP_LOCK/VOP_UNLOCK in tmpfs_reclaim()? It was added to > > prevent from the racy access of tn_links. > > Hopefully a vnode lock is needed to access tn_links -- otherwise the > VOP_LOCK/UNLOCK in tmpfs_reclaim() didn't protect it ... Yes, tn_links is modified with the vnode lock held. Unlocked access in VOP_RECLAIM() racing with the release of the last last reference, used to cause crashes due to premature reclamation attempts. Given that VOP_LOCK() acquires the vnode node, can you explain why this "didn't protect it"? > As this commit prevents other threads from obtaining a lock while > VOP_RECLAIM() is running the protection should be the same as before. You mean VOP_RECLAIM() is now called with the vnode lock held? Can you please add an assert in tmpfs_reclaim() which demonstrates that? Thanks. -- Mindaugas
Re: CVS commit: src
On Feb 27, 2014, at 7:01 PM, Mindaugas Rasiukevicius wrote: > "J. Hannken-Illjes" wrote: >>> I have not had time to follow your VFS changes, but can you explain why >>> did you remove VOP_LOCK/VOP_UNLOCK in tmpfs_reclaim()? It was added to >>> prevent from the racy access of tn_links. >> >> Hopefully a vnode lock is needed to access tn_links -- otherwise the >> VOP_LOCK/UNLOCK in tmpfs_reclaim() didn't protect it ... > > Yes, tn_links is modified with the vnode lock held. Unlocked access in > VOP_RECLAIM() racing with the release of the last last reference, used to > cause crashes due to premature reclamation attempts. Given that VOP_LOCK() > acquires the vnode node, can you explain why this "didn't protect it"? As stated above I hoped tn_links is modified only with the lock held. >> As this commit prevents other threads from obtaining a lock while >> VOP_RECLAIM() is running the protection should be the same as before. > > You mean VOP_RECLAIM() is now called with the vnode lock held? Can you > please add an assert in tmpfs_reclaim() which demonstrates that? No -- while VOP_RECLAIM() is called no thread may get the lock which is the same as if it were held here. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
re: CVS commit: src/distrib/sets/lists/comp
Martin Husemann writes: > On Thu, Feb 27, 2014 at 10:07:36AM +0100, J. Hannken-Illjes wrote: > > ./usr/include/g++/bits/sparc > > ./usr/include/g++/bits/sparc64 > > I asked Jörg to try (his MKCOMPAT=no clang build) with > > ./usr/include/g++/bits/sparc gcc,compat > ./usr/include/g++/bits/sparc64gcc > > (the old state was obviosly incorrect, but so is the current one). > This is still incorrect for gcc 4.1, but I bet noone will notice. as i said elsewhere -- the right solution here is to put those files into new mtree files in src/etc/mtree, and avoid creating them for the builds that "don't want them". (personally, i'd just always create them and have them be empty and ignored for any sort of build that doesn't put files in there. putting any marker (like gcc, or compat) on these subdirs means that they'll always be removed on some build types, ie, that means that the obsolete phase will *always* do something for a fresh build. .mrg.
Re: CVS commit: src/distrib/sets/lists/comp
On Fri, Feb 28, 2014 at 06:34:10AM +1100, matthew green wrote: > putting any marker (like gcc, or compat) on these subdirs means > that they'll always be removed on some build types, ie, that > means that the obsolete phase will *always* do something for a > fresh build. But they are only created with MKCOMPAT=yes (the mtree file is used conditionally), so at least the compat as marker is needed (this is the problem that triggered the original change). Martin
re: CVS commit: src/distrib/sets/lists/comp
> putting any marker (like gcc, or compat) on these subdirs means > that they'll always be removed on some build types, ie, that > means that the obsolete phase will *always* do something for a > fresh build. actually, they should be marked just "compat" as they're only created for compat builds. .mrg.
Re: CVS commit: src/share/mk
On Feb 27, 2014, at 12:43 PM, matthew green wrote: > remove the GCC 4 EXTERNAL_GCC_SUBDIR, and switch GCC 4.8 to use gcc.old. huh?
Re: CVS commit: src/distrib/sets/lists/comp
On Feb 27, 2014, at 4:56 PM, Joerg Sonnenberger wrote: > Module Name: src > Committed By: joerg > Date: Thu Feb 27 15:56:12 UTC 2014 > > Modified Files: > src/distrib/sets/lists/comp: md.sparc64 > > Log Message: > Use gcc,compat, directory creation only depends on the MKCOMPAT switch. > > > To generate a diff of this commit: > cvs rdiff -u -r1.180 -r1.181 src/distrib/sets/lists/comp/md.sparc64 Building sparc64 release now gives: == 2 missing files in DESTDIR Files in flist but missing from DESTDIR. File wasn't installed ? -- ./usr/include/g++/bits/sparc/c++config.h ./usr/include/g++/bits/sparc64/c++config.h end of 2 missing files == -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/uvm
On Wed, 26 Feb 2014, Matt Thomas wrote: Modified Files: src/sys/uvm: uvm_meter.c uvm_param.h Log Message: Add vm.min_address and vm.max_address which return VM_MIN_ADDRESS and VM_MAXUSER_ADDRESS. Do these need to use the old style #define constants instead of the new style (since 2005) CTL_CREATE idiom? --apb (Alan Barrett)
Re: CVS commit: src/sys/dev/pci
On Thu, 27 Feb 2014, Joerg Sonnenberger wrote: Modified Files: src/sys/dev/pci: if_ti.c Log Message: Remove impossible checks. If new bugs are introduced in the future, then these tests might no longer be impossible. So I'd prefer to change them to KASSERT or KDASSERT rather than deleting them. --apb (Alan Barrett)
