Re: CVS commit: src
> On 3. Feb 2024, at 19:45, Christos Zoulas wrote: > > Oh, I fixed GETARGS, but not UPDATE... How about now? What is the expected result if we update a cd9660 mount with different uid/gid or masks? If it is forbidden the "mp->mnt_flag & MNT_UPDATE" case, lines 306 .. 312 of cd9660_vfsops.c should test them and fail on different values. If it is allowed to change them it has to be done here. >> On Feb 3, 2024, at 12:48 PM, J. Hannken-Illjes wrote: >> >>> On 3. Feb 2024, at 18:38, Christos Zoulas wrote: >>> >>> Fixed, thanks. >> >> Not really, with "struct iso_args *args = data" you cannot set fields >> beyond "datalen" here and the MNT_UPDATE case is still missing. -- J. Hannken-Illjes - hann...@mailbox.org
Re: CVS commit: src
> On 3. Feb 2024, at 18:38, Christos Zoulas wrote: > > Fixed, thanks. Not really, with "struct iso_args *args = data" you cannot set fields beyond "datalen" here and the MNT_UPDATE case is still missing. -- J. Hannken-Illjes - hann...@mailbox.org
Re: CVS commit: src
> On 2. Feb 2024, at 21:27, Christos Zoulas wrote: > > Module Name: src > Committed By: christos > Date: Fri Feb 2 20:27:26 UTC 2024 > > Modified Files: > src/sbin/mount_cd9660: Makefile mount_cd9660.8 mount_cd9660.c > src/sys/fs/cd9660: cd9660_extern.h cd9660_mount.h cd9660_vfsops.c >cd9660_vnops.c > > Log Message: > PR/57897: Ricardo Branco: Add support for mount options mask,dirmask,uid,gid > > > To generate a diff of this commit: > cvs rdiff -u -r1.12 -r1.13 src/sbin/mount_cd9660/Makefile > cvs rdiff -u -r1.31 -r1.32 src/sbin/mount_cd9660/mount_cd9660.8 > cvs rdiff -u -r1.33 -r1.34 src/sbin/mount_cd9660/mount_cd9660.c > cvs rdiff -u -r1.27 -r1.28 src/sys/fs/cd9660/cd9660_extern.h > cvs rdiff -u -r1.6 -r1.7 src/sys/fs/cd9660/cd9660_mount.h > cvs rdiff -u -r1.97 -r1.98 src/sys/fs/cd9660/cd9660_vfsops.c > cvs rdiff -u -r1.62 -r1.63 src/sys/fs/cd9660/cd9660_vnops.c First, this is not backwards compatible. An old mount_cd9660 and a new kernel will always return EINVAL as the argument length check on top of cd9660_mount() fires. Second, with these new mount arguments an update mount has to update the masks and ids, currently they are silently ignored. -- J. Hannken-Illjes - hann...@mailbox.org
Re: CVS commit: src/sys
> On 10. Oct 2023, at 20:58, Andrew Doran wrote: > > On Tue, Oct 10, 2023 at 06:00:57PM +0200, J. Hannken-Illjes wrote: > >>> cvs rdiff -u -r1.63 -r1.64 src/sys/kern/sys_select.c >> >> -sleepq_unsleep(l, false); >> +sleepq_remove(l->l_sleepq, l, true); >>} >> } >> mutex_spin_exit(lock); >> >> Looks like sleepq_remove() unlocks l->l_mutex == lock and >> then mutex_spin_exit(lock) will unlock an unlocked mutex. > > lock is held before the call to sleepq_remove() and this is also true at the > time: l->l_mutex == lock. > > After the call lock is still held, but now l->l_mutex != lock, because l has > changed state (e.g LSSLEEP -> LSRUN) which causes l_mutex to change in > concert. There is a rough overview here: > > https://nxr.netbsd.org/xref/src/sys/kern/kern_lwp.c#156 > > Did you encounter a problem? This is not true for RUMP. Hero you added sleepq_remove() as "sleepq_unsleep(l, true)". This will unlock l_mutex without changing. Just poking around and using sleepq_unsleep(l, false) here makes the NFS tests using rump_server pass. -- J. Hannken-Illjes - hann...@mailbox.org signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/sys
> On 8. Oct 2023, at 15:23, Andrew Doran wrote: > > Module Name: src > Committed By: ad > Date: Sun Oct 8 13:23:05 UTC 2023 > > Modified Files: > src/sys/kern: kern_condvar.c kern_sleepq.c kern_timeout.c >kern_turnstile.c sys_lwp.c sys_select.c > src/sys/rump/librump/rumpkern: sleepq.c > src/sys/sys: sleepq.h syncobj.h > > Log Message: > Ensure that an LWP that has taken a legitimate wakeup never produces an > error code from sleepq_block(). Then, it's possible to make cv_signal() > work as expected and only ever wake a singular LWP. > > > To generate a diff of this commit: ... > cvs rdiff -u -r1.63 -r1.64 src/sys/kern/sys_select.c -sleepq_unsleep(l, false); +sleepq_remove(l->l_sleepq, l, true); } } mutex_spin_exit(lock); Looks like sleepq_remove() unlocks l->l_mutex == lock and then mutex_spin_exit(lock) will unlock an unlocked mutex. -- J. Hannken-Illjes - hann...@mailbox.org signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/external/cddl/osnet/dist/uts/common/fs/zfs
Yes -- I suppose genfs_pathconf() is sufficient here. -- J. Hannken-Illjes - hann...@mailbox.org > On 3. Oct 2022, at 11:40, Frank Kardel wrote: > > > Good to know. So do we need to add path conf there? > Frank > > 3 Oct 2022 11:34:09 J. Hannken-Illjes : > >> Frank, >> >> the vnode operations for ".zfs" are in zfs_ctldir.c and there >> is no pathconf operation here ... >> >> -- >> J. Hannken-Illjes - hann...@mailbox.org >> >>> On 3. Oct 2022, at 11:26, Frank Kardel wrote: >>> >>> Well, I am am happy to do that. But ls got eopnotsupp for fpathconf of ACL >>> names and barked at them when listing the .zfs snapshot directory. Ls is >>> only ignoring einval at that place. So something must be wrong there wrt >>> tog. >>> Frank >>> >>> 3 Oct 2022 10:53:10 J. Hannken-Illjes : >>> >>>>> On 27. Sep 2022, at 12:33, Frank Kardel wrote: >>>>> >>>>> Module Name:src >>>>> Committed By: kardel >>>>> Date: Tue Sep 27 10:33:21 UTC 2022 >>>>> >>>>> Modified Files: >>>>> src/external/cddl/osnet/dist/uts/common/fs/zfs: zfs_vnops.c >>>>> >>>>> Log Message: >>>>> for unsupported names return EINVAL as per TOG >>>>> https://pubs.opengroup.org/onlinepubs/9699919799/functions/fpathconf.html >>>>> discussed with christos@ >>>>> >>>>> >>>>> To generate a diff of this commit: >>>>> cvs rdiff -u -r1.78 -r1.79 \ >>>>>src/external/cddl/osnet/dist/uts/common/fs/zfs/zfs_vnops.c >>>> >>>> This is completely wrong! >>>> >>>> The call sequence is "VOP_PATHCONF -> zfs_netbsd_pathconf -> zfs_pathconf" >>>> where zfs_netbsd_pathconf() handles the NetBSD specific names if >>>> zfs_pathconf() returns EOPNOTSUPP and also returns EINVAL for >>>> unsupported names in this case. >>>> >>>> Please revert. >>>> >>>> -- >>>> J. Hannken-Illjes - hann...@mailbox.org signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/external/cddl/osnet/dist/uts/common/fs/zfs
Frank, the vnode operations for ".zfs" are in zfs_ctldir.c and there is no pathconf operation here ... -- J. Hannken-Illjes - hann...@mailbox.org > On 3. Oct 2022, at 11:26, Frank Kardel wrote: > > Well, I am am happy to do that. But ls got eopnotsupp for fpathconf of ACL > names and barked at them when listing the .zfs snapshot directory. Ls is only > ignoring einval at that place. So something must be wrong there wrt tog. > Frank > > 3 Oct 2022 10:53:10 J. Hannken-Illjes : > >>> On 27. Sep 2022, at 12:33, Frank Kardel wrote: >>> >>> Module Name:src >>> Committed By: kardel >>> Date: Tue Sep 27 10:33:21 UTC 2022 >>> >>> Modified Files: >>> src/external/cddl/osnet/dist/uts/common/fs/zfs: zfs_vnops.c >>> >>> Log Message: >>> for unsupported names return EINVAL as per TOG >>> https://pubs.opengroup.org/onlinepubs/9699919799/functions/fpathconf.html >>> discussed with christos@ >>> >>> >>> To generate a diff of this commit: >>> cvs rdiff -u -r1.78 -r1.79 \ >>>src/external/cddl/osnet/dist/uts/common/fs/zfs/zfs_vnops.c >> >> This is completely wrong! >> >> The call sequence is "VOP_PATHCONF -> zfs_netbsd_pathconf -> zfs_pathconf" >> where zfs_netbsd_pathconf() handles the NetBSD specific names if >> zfs_pathconf() returns EOPNOTSUPP and also returns EINVAL for >> unsupported names in this case. >> >> Please revert. >> >> -- >> J. Hannken-Illjes - hann...@mailbox.org signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/external/cddl/osnet/dist/uts/common/fs/zfs
> On 27. Sep 2022, at 12:33, Frank Kardel wrote: > > Module Name: src > Committed By: kardel > Date: Tue Sep 27 10:33:21 UTC 2022 > > Modified Files: > src/external/cddl/osnet/dist/uts/common/fs/zfs: zfs_vnops.c > > Log Message: > for unsupported names return EINVAL as per TOG > https://pubs.opengroup.org/onlinepubs/9699919799/functions/fpathconf.html > discussed with christos@ > > > To generate a diff of this commit: > cvs rdiff -u -r1.78 -r1.79 \ >src/external/cddl/osnet/dist/uts/common/fs/zfs/zfs_vnops.c This is completely wrong! The call sequence is "VOP_PATHCONF -> zfs_netbsd_pathconf -> zfs_pathconf" where zfs_netbsd_pathconf() handles the NetBSD specific names if zfs_pathconf() returns EOPNOTSUPP and also returns EINVAL for unsupported names in this case. Please revert. -- J. Hannken-Illjes - hann...@mailbox.org signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/sys
> On 4. Jun 2022, at 05:31, Paul Goyette wrote: > > Module Name: src > Committed By: pgoyette > Date: Sat Jun 4 03:31:10 UTC 2022 > > Modified Files: > src/sys/dev: files.audio files.dev midi.c sequencer.c > src/sys/modules: Makefile > src/sys/modules/midi: Makefile > src/sys/modules/sequencer: Makefile > Added Files: > src/sys/dev: midi_mod.c midi_seq_mod.c sequencer_mod.c > src/sys/modules/midi_seq: Makefile midi_seq.ioconf > Removed Files: > src/sys/modules/midi: midi.ioconf > src/sys/modules/sequencer: sequencer.ioconf > > Log Message: > Combine the midi and sequencer modules into a single midi_seq module > to avoid a circular dependency as noted in kern/56772. Retain minimal > modules of the original names to accomodate auto-loading upon access > to the /dev/xxx nodes. This breaks at least sparc64/GENERIC: sys/dev/sequencer.c:285:1: error: no previous prototype for 'sequencerattach' [-Werror=missing-prototypes] 285 | sequencerattach(int n) | ^~~ It has "midi* at midibus?" but no "pseudo-device sequencer". -- J. Hannken-Illjes - hann...@mailbox.org signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src
> On 29. May 2022, at 19:04, nia wrote: > > On Sun, May 29, 2022 at 05:50:30PM +0100, Alistair Crooks wrote: >> But before starting to make piecemeal changes to the build system, maybe >> some research into how others have accomplished this before with NetBSD? >> >> I'd really like to avoid the plethora of (sometimes conflicting) switches >> that make up the FreeBSD build system. > > Hmm, okay. I wouldn't like to introduce any conflicting options, > and I'd like to keep the changes as unintrusive as possible, right > now only modifying the make system and adding flags to the set lists. > >> And I'd like to see discussion before any work is done (on a branch for >> such intrusive changes) for this > > OK, should I back out my changes? Just a side note, how do we test a build system with say 20 knobs, do we build all 2**20 configurations to be sure everything at least builds? Isn't it better to always build everything and move this selection into the set lists or whatever you use to get the final image? -- J. Hannken-Illjes - hann...@mailbox.org signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/sys/dev/raidframe
> On 16. Apr 2022, at 18:40, Andrius Varanavicius wrote: > > Module Name: src > Committed By: andvar > Date: Sat Apr 16 16:40:54 UTC 2022 > > Modified Files: > src/sys/dev/raidframe: rf_netbsdkintf.c > > Log Message: > Fix mistake in error branch locking caused by previous changes. > vput(vp) also unlocks vp, thus unlocking happens twice in error flow > causing kernel to panic with failed assertion lktype != LK_NONE > in vfs_vnode.c#778. Thanks riastradh with finding the issue. > > > To generate a diff of this commit: > cvs rdiff -u -r1.406 -r1.407 src/sys/dev/raidframe/rf_netbsdkintf.c > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. Thanks, replacing vput() with vrele() would have been even better ... -- J. Hannken-Illjes - hann...@mailbox.org signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/etc
> On 4. Feb 2022, at 23:19, Alexander Nasonov wrote: > > Martin Husemann wrote: >> On Thu, Feb 03, 2022 at 11:10:43PM +, Alexander Nasonov wrote: >>> variable, it will mix two very different styles of mounting and >>> compilate the code. > > s/compilate/complicate/ > >> "different styles of mounting" sounds like a non-starter to me, maybe >> that should be fixed first? > > These two "styles of mounting" are > > /sbin/mount /filesystem - looks up fs parameters in /etc/fstab > /sbin/zfs mount dataset - looks up fs parameters in zpools > > I don't think these two approaches can be unified. What is wrong with ZFS legacy mounts? $ zpool create -m legacy tank $ zfs create tank/a $ mount -t zfs tank/a /mnt > We surely > can modify mount_critical_systems to try entries from /etc/fstab > first, and if /sbin/mount fails, then try to find a zfs dataset > for the failed entry and /sbin/zfs mount it. But if things go > wrong, a complicated mounting process will make troubleshooting > harder. For that reason, I'd like to keep mountcrit_zfs separate > from mountcrit_local. -- J. Hannken-Illjes - hann...@mailbox.org signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/sys
Jared, please test if the attached diff is sufficient. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig uvm_swap.c.diff Description: Binary data > On 17. Feb 2021, at 13:07, Jared McNeill wrote: > > I noticed this on reboot since this change: > > [ 3636.2891122] turning off swap...stopping swap on /swap failed with error 16 > [ 3636.2991109] stopping swap on /swap2 failed with error 16 > > Can you have a look? > > Thanks! > Jared > > > On Tue, 16 Feb 2021, Juergen Hannken-Illjes wrote: > >> Module Name: src >> Committed By:hannken >> Date:Tue Feb 16 09:56:32 UTC 2021 >> >> Modified Files: >> src/sys/kern: vfs_mount.c >> src/sys/uvm: uvm_swap.c >> >> Log Message: >> Reorganize uvm_swap_shutdown() a bit, make sure the vnode gets >> locked and referenced across the call to swap_off() and finally >> use it from vfs_unmountall1() to remove swap after unmounting >> the last file system. >> >> Adresses PR kern/54969 (Disk cache is no longer flushed on shutdown) >> >> >> To generate a diff of this commit: >> cvs rdiff -u -r1.85 -r1.86 src/sys/kern/vfs_mount.c >> cvs rdiff -u -r1.200 -r1.201 src/sys/uvm/uvm_swap.c >> >> Please note that diffs are not public domain; they are subject to the >> copyright notices on the relevant files. >> >> signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/sys/dev/pci
> On 12. Jul 2020, at 21:05, Jaromir Dolecek wrote: > > Module Name: src > Committed By: jdolecek > Date: Sun Jul 12 19:05:32 UTC 2020 > > Modified Files: > src/sys/dev/pci: if_bnx.c if_bnxvar.h > > Log Message: > enable MSI/MSI-X if supported by adapter > > tested MSI-X with Broadcom NetXtreme II BCM5709 1000Base-T > > > To generate a diff of this commit: > cvs rdiff -u -r1.95 -r1.96 src/sys/dev/pci/if_bnx.c > cvs rdiff -u -r1.12 -r1.13 src/sys/dev/pci/if_bnxvar.h > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. With this change my Dell PowerEdge 2850 spits watchdog resets: [ 68.1828359] bnx0: Watchdog timeout -- resetting! [ 88.6042909] bnx0: Watchdog timeout -- resetting! [ 119.0265230] bnx0: Watchdog timeout -- resetting! [ 145.4484562] bnx0: Watchdog timeout -- resetting! Dmesg before is: [ 1.017306] pci4 at ppb3 bus 7 [ 1.017306] pci4: i/o space, memory space enabled, rd/line, wr/inv ok [ 1.017306] bnx0 at pci4 dev 0 function 0: Broadcom NetXtreme II BCM5708 1000Base-T [ 1.017306] bnx0: Ethernet address 00:24:e8:67:4b:db [ 1.017306] bnx0: ASIC BCM5708 B2 (0x57081020) [ 1.017306] bnx0: PCI-X 64bit 133MHz [ 1.017306] bnx0: B/C (4.6.0); Bufs (RX:2;TX:2); Flags (MFW); MFW (ipms 1.6.0) [ 1.017306] bnx0: Coal (RX:6,6,18,18; TX:20,20,80,80) [ 1.017306] brgphy0 at bnx0 phy 1: BCM5708C 1000BASE-T media interface, rev. 6 [ 1.017306] brgphy0: 10baseT, 10baseT-FDX, 100baseTX, 100baseTX-FDX, 1000baseT, 1000baseT-FDX, auto [ 1.017306] bnx0: interrupting at ioapic0 pin 16 while dmesg after is: [ 1.017262] pci4 at ppb3 bus 7 [ 1.017262] pci4: i/o space, memory space enabled, rd/line, wr/inv ok [ 1.017262] bnx0 at pci4 dev 0 function 0: Broadcom NetXtreme II BCM5708 1000Base-T [ 1.017262] bnx0: Ethernet address 00:24:e8:67:4b:db [ 1.017262] bnx0: ASIC BCM5708 B2 (0x57081020) [ 1.017262] bnx0: PCI-X 64bit 133MHz [ 1.017262] bnx0: B/C (4.6.0); Bufs (RX:2;TX:2); Flags (MFW); MFW (ipms 1.6.0) [ 1.017262] bnx0: Coal (RX:6,6,18,18; TX:20,20,80,80) [ 1.017262] brgphy0 at bnx0 phy 1: BCM5708C 1000BASE-T media interface, rev. 6 [ 1.017262] brgphy0: 10baseT, 10baseT-FDX, 100baseTX, 100baseTX-FDX, 1000baseT, 1000baseT-FDX, auto [ 1.017262] bnx0: interrupting at msi0 vec 0 Pcictl dump gives (in the MSI-X case): PCI Message Signaled Interrupt Message Control register: 0x0081 MSI Enabled: on Multiple Message Capable: no (1 vector) Multiple Message Enabled: off (1 vector) 64 Bit Address Capable: on Per-Vector Masking Capable: off Extended Message Data Capable: off Extended Message Data Enable: off Message Address (lower) register: 0xfee0 Message Address (upper) register: 0x Message Data register: 0x0064 Any ideas how to fix this issue? -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany) signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/sys/rump/include/rump
> On 15. Jun 2020, at 01:38, Kamil Rytarowski wrote: > > Module Name: src > Committed By: kamil > Date: Sun Jun 14 23:38:25 UTC 2020 > > Modified Files: > src/sys/rump/include/rump: rump.h > > Log Message: > Remove old compat include of rump_syscallshotgun.h > > It was separated in 2016 and is no longer needed. > > > To generate a diff of this commit: > cvs rdiff -u -r1.71 -r1.72 src/sys/rump/include/rump/rump.h > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. This broke most or all NFS tests on ATF (see attached list). Please revert or investigate. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany) fs/nfs/t_mountd::mountdhup fs/vfs/t_full::nfs_fillfs fs/vfs/t_io::nfs_extendfile fs/vfs/t_io::nfs_extendfile_append fs/vfs/t_io::nfs_holywrite fs/vfs/t_io::nfs_overwrite512 fs/vfs/t_io::nfs_overwrite64k fs/vfs/t_io::nfs_overwrite_trunc fs/vfs/t_io::nfs_read_after_unlink fs/vfs/t_io::nfs_read_fault fs/vfs/t_io::nfs_shrinkfile fs/vfs/t_io::nfs_wrrd_after_unlink fs/vfs/t_mtime_otrunc::nfs_otrunc_mtime_update fs/vfs/t_mtime_write::nfs_mtime_update_on_write fs/vfs/t_renamerace::nfs_renamerace fs/vfs/t_renamerace::nfs_renamerace_dirs fs/vfs/t_rmdirrace::nfs_race fs/vfs/t_ro::nfs_attrs fs/vfs/t_ro::nfs_create fs/vfs/t_ro::nfs_createdir fs/vfs/t_ro::nfs_createfifo fs/vfs/t_ro::nfs_createlink fs/vfs/t_ro::nfs_createsymlink fs/vfs/t_ro::nfs_fileio fs/vfs/t_ro::nfs_rmfile fs/vfs/t_ro::nfsro_attrs fs/vfs/t_ro::nfsro_create fs/vfs/t_ro::nfsro_createdir fs/vfs/t_ro::nfsro_createfifo fs/vfs/t_ro::nfsro_createlink fs/vfs/t_ro::nfsro_createsymlink fs/vfs/t_ro::nfsro_fileio fs/vfs/t_ro::nfsro_rmfile fs/vfs/t_rwtoro::nfs_layer_noneopen fs/vfs/t_rwtoro::nfs_layer_read_unlinked fs/vfs/t_rwtoro::nfs_layer_readopen fs/vfs/t_rwtoro::nfs_layer_writeopen fs/vfs/t_rwtoro::nfs_noneopen fs/vfs/t_rwtoro::nfs_read_unlinked fs/vfs/t_rwtoro::nfs_readopen fs/vfs/t_rwtoro::nfs_writeopen fs/vfs/t_union::nfs_basic fs/vfs/t_union::nfs_whiteout fs/vfs/t_unpriv::nfs_dirperms fs/vfs/t_unpriv::nfs_flags fs/vfs/t_unpriv::nfs_owner fs/vfs/t_unpriv::nfs_times fs/vfs/t_vfsops::nfs_tfhinval fs/vfs/t_vfsops::nfs_tfhremove fs/vfs/t_vfsops::nfs_tfilehandle fs/vfs/t_vfsops::nfs_tmount fs/vfs/t_vfsops::nfs_tstatvfs fs/vfs/t_vfsops::nfs_tsync fs/vfs/t_vnops::nfs_access_simple fs/vfs/t_vnops::nfs_attrs fs/vfs/t_vnops::nfs_create_exist fs/vfs/t_vnops::nfs_create_many fs/vfs/t_vnops::nfs_create_nametoolong fs/vfs/t_vnops::nfs_create_nonalphanum fs/vfs/t_vnops::nfs_dir_notempty fs/vfs/t_vnops::nfs_dir_rmdirdotdot fs/vfs/t_vnops::nfs_dir_simple fs/vfs/t_vnops::nfs_fcntl_getlock_pids fs/vfs/t_vnops::nfs_fcntl_lock fs/vfs/t_vnops::nfs_lookup_complex fs/vfs/t_vnops::nfs_lookup_simple fs/vfs/t_vnops::nfs_lstat_symlink fs/vfs/t_vnops::nfs_read_directory fs/vfs/t_vnops::nfs_rename_dir fs/vfs/t_vnops::nfs_rename_dotdot fs/vfs/t_vnops::nfs_rename_nametoolong fs/vfs/t_vnops::nfs_rename_reg_nodir fs/vfs/t_vnops::nfs_symlink_long fs/vfs/t_vnops::nfs_symlink_root fs/vfs/t_vnops::nfs_symlink_zerolen signature.asc Description: Message signed with OpenPGP
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
> On 22. Mar 2020, at 14:30, Paul Goyette wrote: > > Module Name: src > Committed By: pgoyette > Date: Sun Mar 22 13:30:11 UTC 2020 > > Modified Files: > src/lib/librumpuser: rumpuser_dl.c > src/sys/rump/include/rump: rumpuser.h > src/sys/rump/librump/rumpkern: rump.c > > Log Message: > Teach rump to process __link_set_evcnts entries. (Second part of > fix for PR kern/55088) > > > To generate a diff of this commit: > cvs rdiff -u -r1.32 -r1.33 src/lib/librumpuser/rumpuser_dl.c > cvs rdiff -u -r1.115 -r1.116 src/sys/rump/include/rump/rumpuser.h > cvs rdiff -u -r1.342 -r1.343 src/sys/rump/librump/rumpkern/rump.c > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. This gives me 885 failed test cases for tests/fs: Core was generated by `t_vnops'. Program terminated with signal SIGSEGV, Segmentation fault. #0 0x70040647dac3 in evcnt_detach (ev=0x700404e031c0) at src/lib/librump/../../sys/rump/../kern/subr_evcnt.c:193 #1 0x7004064b93da in add_static_evcnt (ev=0x700404e031c0) at src/lib/librump/../../sys/rump/librump/rumpkern/rump.c:657 #2 0x70040600442a in process_object ( doevcntattach=0x7004064b93d1 , docompload=0x7004064b93f1 , domodinit=0x7004064b93e3 , handle=0x70040bbd9000) at src/lib/librumpuser/rumpuser_dl.c:392 #3 rumpuser_dl_bootstrap ( domodinit=domodinit@entry=0x7004064b93e3 , symload=symload@entry=0x7004064b94e2 , compload=compload@entry=0x7004064b93f1 , doevcntattach=doevcntattach@entry=0x7004064b93d1 ) at src/lib/librumpuser/rumpuser_dl.c:499 #4 0x7004064b9b5f in rump_init () at src/lib/librump/../../sys/rump/librump/rumpkern/rump.c:426 Are sure it is ok to detach a non-attached event? -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/external/cddl/osnet/dist/uts/common/fs/zfs
> On 10. Mar 2020, at 13:37, Santhosh Raju wrote: > > On Tue, Mar 10, 2020 at 7:25 AM Santhosh Raju wrote: >> >> Module Name:src >> Committed By: fox >> Date: Mon Mar 9 15:40:50 UTC 2020 >> >> Modified Files: >>src/external/cddl/osnet/dist/uts/common/fs/zfs: metaslab.c >> >> Log Message: >> external/cddl/osnet: Fix possible null pointer access. >> >> Detected by UBSan and fixed upstream, pick only the fix from the commit. >> >> Cherry-pick: >> From 928e8ad47d3478a3d5d01f0dd6ae74a9371af65e Mon Sep 17 00:00:00 2001 >> From: Serapheim Dimitropoulos >> Date: Wed, 20 Feb 2019 09:59:57 -0800 >> Subject: [PATCH] Introduce auxiliary metaslab histograms >> >> Reviewed by: Paul Dagnelie >> Reviewed-by: Brian Behlendorf >> Reviewed by: Matt Ahrens >> Signed-off-by: Serapheim Dimitropoulos >> Closes #8358 >> >> Reviewed by: kamil@ >> >> >> To generate a diff of this commit: >> cvs rdiff -u -r1.1.1.3 -r1.2 \ >>src/external/cddl/osnet/dist/uts/common/fs/zfs/metaslab.c >> >> Please note that diffs are not public domain; they are subject to the >> copyright notices on the relevant files. >> > > Forgot to add in the commit log, the changes have been pulled in from > upstream openzfs. > > https://github.com/openzfs/zfs/commit/928e8ad47d3478a3d5d01f0dd6ae74a9371af65e#diff-9fd6b453f8153161abe0728c449e6505R4386 > > -- > Santhosh This is NOT our upstream -- -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/sys/kern
> On Feb 17, 2020, at 11:19 PM, Andrew Doran wrote:
>
> Hi,
>
> On Thu, Feb 06, 2020 at 06:33:55PM +0100, J. Hannken-Illjes wrote:
>
>>> On 12. Jan 2020, at 18:49, Andrew Doran wrote:
>>>
>>> Module Name:src
>>> Committed By: ad
>>> Date: Sun Jan 12 17:49:17 UTC 2020
>>>
>>> Modified Files:
>>> src/sys/kern: vfs_vnode.c
>>>
>>> Log Message:
>>> vput(): don't drop the vnode lock, carry the hold over into vrelel() which
>>> might need it anyway.
>>>
>>>
>>> To generate a diff of this commit:
>>> cvs rdiff -u -r1.106 -r1.107 src/sys/kern/vfs_vnode.c
>>>
>>> Please note that diffs are not public domain; they are subject to the
>>> copyright notices on the relevant files.
>>>
>>
>> vput(vnode_t *vp)
>> {
>> + int lktype;
>>
>> - VOP_UNLOCK(vp);
>> - vrele(vp);
>> + if ((vp->v_vflag & VV_LOCKSWORK) == 0) {
>> + lktype = LK_EXCLUSIVE;
>> + } else {
>> + lktype = VOP_ISLOCKED(vp);
>> + KASSERT(lktype != LK_NONE);
>> + }
>> + mutex_enter(vp->v_interlock);
>> + vrelel(vp, 0, lktype);
>> }
>>
>> This is quite wrong, from the manual:
>>
>> VOP_ISLOCKED(vp)
>> Test if the vnode vp is locked. Possible return values are
>> LK_EXCLUSIVE, LK_SHARED or 0 for lock held exclusively by the
>> calling thread, shared lock held by anyone or unlocked,
>> respectively.
>>
>> This function must never be used to make locking decisions at
>> run time: it is provided only for diagnostic purposes.
>>
>> I suppose you cannot determine if the current thread holds
>> a shared lock.
>
> The intention of that last sentence was to dissuade people from doing error
> prone, complicated stuff with locks. To my mind that idea of the locking
> primitives is to take something difficult (concurrency) and package it up in
> a way that's much easier to think about and work with - and yes it's still
> complicated.
>
> There are limited cases when I think it makes sense to infer lock ownership,
> for example when known for sure that a RW lock is held but the type of hold
> needs to be known - like this case. The failure case there is the lock
> being unheld, which would be caught by LOCKDEBUG in this case. Consider
> that a rw_exit() with the lock unheld by the caller will produce what you
> might charitably call "undefined behaviour" in a non-LOCKDEBUG kernel.
>
> Andrew
Yes, I think it is safe here but it deserves a comment like the text above.
--
J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
signature.asc
Description: Message signed with OpenPGP
Re: CVS commit: src/sys/kern
> On 12. Jan 2020, at 18:49, Andrew Doran wrote:
>
> Module Name: src
> Committed By: ad
> Date: Sun Jan 12 17:49:17 UTC 2020
>
> Modified Files:
> src/sys/kern: vfs_vnode.c
>
> Log Message:
> vput(): don't drop the vnode lock, carry the hold over into vrelel() which
> might need it anyway.
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.106 -r1.107 src/sys/kern/vfs_vnode.c
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
>
vput(vnode_t *vp)
{
+ int lktype;
- VOP_UNLOCK(vp);
- vrele(vp);
+ if ((vp->v_vflag & VV_LOCKSWORK) == 0) {
+ lktype = LK_EXCLUSIVE;
+ } else {
+ lktype = VOP_ISLOCKED(vp);
+ KASSERT(lktype != LK_NONE);
+ }
+ mutex_enter(vp->v_interlock);
+ vrelel(vp, 0, lktype);
}
This is quite wrong, from the manual:
VOP_ISLOCKED(vp)
Test if the vnode vp is locked. Possible return values are
LK_EXCLUSIVE, LK_SHARED or 0 for lock held exclusively by the
calling thread, shared lock held by anyone or unlocked,
respectively.
This function must never be used to make locking decisions at
run time: it is provided only for diagnostic purposes.
I suppose you cannot determine if the current thread holds
a shared lock.
--
J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig
signature.asc
Description: Message signed with OpenPGP
Re: CVS commit: src/sys/miscfs/nullfs
> On 15. Dec 2019, at 21:30, Joerg Sonnenberger wrote: > > Module Name: src > Committed By: joerg > Date: Sun Dec 15 20:30:56 UTC 2019 > > Modified Files: > src/sys/miscfs/nullfs: null_vfsops.c > > Log Message: > Set IMNT_MPSAFE before creating the vnode for the root of the > filesystem. Otherwise, it won't be created with VV_MPSAFE and require > the kernel_lock. > > > To generate a diff of this commit: > cvs rdiff -u -r1.95 -r1.96 src/sys/miscfs/nullfs/null_vfsops.c > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. Great! Please request pullups to -8 and -9. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/external/cddl/osnet/dist/uts/common/fs/zfs
Should be fixed with Rev. 1.4 of src/external/cddl/osnet/dist/uts/common/fs/zfs/dmu_diff.c -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig > On 15. Oct 2019, at 00:13, Paul Goyette wrote: > > I'm now seeing a build error: > > # compile libzpool/dmu_diff.pico > /build/netbsd-local/tools/x86_64/amd64/bin/x86_64--netbsd-gcc -O2 > -std=gnu99-Wall -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith > -Wno-sign-compare -Wsystem-headers -Wno-traditional -Wreturn-type > -Wswitch -Wshadow -Wcast-qual -Wwrite-strings -Wextra -Wno-unused-parameter > -Wno-sign-compare -Wold-style-definition -Wsign-compare -Wformat=2 > -Wno-format-zero-length -Wno-missing-field-initializers > -Wno-strict-prototypes -Wno-cast-qual -Wno-discarded-qualifiers -Wno-switch > -Wno-missing-prototypes -Wno-unused-variable -Wno-shadow > -Wno-missing-field-initializers -Wno-parentheses -fPIE > -Wno-unknown-pragmas -Wno-sign-compare -D_KERNTYPES -D_KERNTYPES > --sysroot=/build/netbsd-local/dest/amd64 -std=c99 -D_SUNOS_VTOC_16 > -D_PROPLIB_ZFS_CONFLICT -I/build/netbsd-local/src_ro/external/cddl/osnet > -I/build/netbsd-local/src_ro/external/cddl/osnet/include > -I/build/netbsd-local/src_ro/external/cddl/osnet/sys > -I/build/netbsd-local/src_ro/external/cddl/osnet/dist/common/zfs > -I/build/netbsd-local/src_ro/external/cddl/osnet/dist/head > -I/build/netbsd-local/src_ro/external/cddl/osnet/dist/lib/libdevinfo > -I/build/netbsd-local/src_ro/external/cddl/osnet/dist/lib/libnvpair > -I/build/netbsd-local/src_ro/external/cddl/osnet/dist/lib/libshare/common > -I/build/netbsd-local/src_ro/external/cddl/osnet/dist/lib/libumem > -I/build/netbsd-local/src_ro/external/cddl/osnet/dist/lib/libuutil/common > -I/build/netbsd-local/src_ro/external/cddl/osnet/dist/lib/libzfs/common > -I/build/netbsd-local/src_ro/external/cddl/osnet/dist/lib/libzfs_core/common > -I/build/netbsd-local/src_ro/external/cddl/osnet/dist/lib/libzpool/common > -I/build/netbsd-local/src_ro/external/cddl/osnet/dist/common > -I/build/netbsd-local/src_ro/external/cddl/osnet/dist/uts/common > -I/build/netbsd-local/src_ro/external/cddl/osnet/dist/uts/common/zfs > -I/build/netbsd-local/src_ro/external/cddl/osnet/dist/uts/common/fs/zfs > -I/build/netbsd-local/src_ro/external/cddl/osnet/dist/uts/common/sys -std=c99 > -c-fPIC -g > /build/netbsd-local/src_ro/external/cddl/osnet/lib/libzpool/../../dist/uts/common/fs/zfs/dmu_diff.c > -o dmu_diff.pico > /build/netbsd-local/src_ro/external/cddl/osnet/lib/libzpool/../../dist/uts/common/fs/zfs/dmu_diff.c: > In function 'write_bytes': > /build/netbsd-local/src_ro/external/cddl/osnet/lib/libzpool/../../dist/uts/common/fs/zfs/dmu_diff.c:70:6: > error: 'struct uio' has no member named 'uio_vmspace' > auio.uio_vmspace = vmspace_kernel(); > ^ > /build/netbsd-local/src_ro/external/cddl/osnet/lib/libzpool/../../dist/uts/common/fs/zfs/dmu_diff.c:70:21: > warning: implicit declaration of function 'vmspace_kernel' > [-Wimplicit-function-declaration] > auio.uio_vmspace = vmspace_kernel(); > ^~ > /build/netbsd-local/src_ro/external/cddl/osnet/lib/libzpool/../../dist/uts/common/fs/zfs/dmu_diff.c:56:13: > warning: variable 'auio' set but not used [-Wunused-but-set-variable] > struct uio auio; > ^~~~ > *** [dmu_diff.pico] Error code 1 > > > > > > ++--+---+ > | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | > | (Retired) | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com | > | Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org | > ++--+---+ signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/tools
> On 22. Jun 2019, at 15:50, Kamil Rytarowski wrote: > > On 22.06.2019 15:42, Juergen Hannken-Illjes wrote: >> Module Name: src >> Committed By:hannken >> Date:Sat Jun 22 13:42:53 UTC 2019 >> >> Modified Files: >> src/tools/compat: configure configure.ac nbtool_config.h.in >> src/tools/rpcgen: Makefile >> >> Log Message: >> Update tools/compat/configure for new path of "rpc/types.h". >> >> Remove intermediate patch from rpcgen/Makefile. >> > > Why intermediate? It worked for me and it was final. Was there anything > wrong with it? > > Other compat tools include in the same way other headers from compat/. > > How is the never version better than the older one? tools/compat/configure installs our "rpc/types.h" if it doesn't find a working one on the host so individual tools don't have to care about the location. > >> @@ -5111,8 +5130,6 @@ else >> # ifdef _MSC_VER >> # include >> # define alloca _alloca >> -# elif defined(__NetBSD__) || defined(__FreeBSD__) || >> defined(__DragonFly__) || defined(__OpenBSD__) >> -# include >> # else >> # ifdef HAVE_ALLOCA_H >> # include > > This looks like unintended change? Fixed. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de <mailto:hann...@eis.cs.tu-bs.de> - TU Braunschweig signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src
> On 1. Jun 2019, at 13:44, Robert Elz wrote: > >Date:Sat, 1 Jun 2019 10:34:41 +0200 >From: "J. Hannken-Illjes" >Message-ID: > > | This looks wrong -- you should have added an weak alias to > | sys/rump/librump/rumpkern/emul.c to handle the case where > | librumpvfs is not present. > > I have no idea how that could have fixed things - if librump > wants to call a function that exists in librumpvfs then librumpvfs > needs to be there doesn't it?Perhaps none of the current > functions need that, because they're probably not going to > exercise the new code, but they could. > > Still, if you have a better way that will actually fix things, by > all means, do it. Problem is sys/kern/kern_proc.c gets loaded into librump while getcwd_common is in librumpvfs. For the librump-only case we have to supply stubs to librumpvfs functions. The attached diff should do the job -- unfortunately my build and test machine is not working aqt the moment. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig emul.c.diff Description: Binary data signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src
> On 1. Jun 2019, at 08:59, Robert Elz wrote: > > Module Name: src > Committed By: kre > Date: Sat Jun 1 06:59:18 UTC 2019 > > Modified Files: > src/tests/dev/usb/t_hid: Makefile > src/tests/lib/semaphore/pthread: Makefile > src/tests/net/bpfjit: Makefile > src/tests/net/icmp: Makefile > src/tests/net/if: Makefile > src/tests/net/if_loop: Makefile > src/tests/rump/rumpkern/h_server: Makefile > src/usr.bin/rump_server: Makefile > > Log Message: > Deal with fallout from the addition of > KERN_PROC_CWD in sysctl(3) > That is kern.proc.$$.KERN_PROC_CWD (I think - not that it matters here) > > The effect is that -lrump now requires -lrumpvfs > > This set of changes fixes (I believe) regular dynamic builds, > more might be required for static builds (will be verified soon). > > > To generate a diff of this commit: > cvs rdiff -u -r1.2 -r1.3 src/tests/dev/usb/t_hid/Makefile > cvs rdiff -u -r1.4 -r1.5 src/tests/lib/semaphore/pthread/Makefile > cvs rdiff -u -r1.7 -r1.8 src/tests/net/bpfjit/Makefile > cvs rdiff -u -r1.10 -r1.11 src/tests/net/icmp/Makefile > cvs rdiff -u -r1.8 -r1.9 src/tests/net/if/Makefile > cvs rdiff -u -r1.5 -r1.6 src/tests/net/if_loop/Makefile > cvs rdiff -u -r1.5 -r1.6 src/tests/rump/rumpkern/h_server/Makefile > cvs rdiff -u -r1.12 -r1.13 src/usr.bin/rump_server/Makefile > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. This looks wrong -- you should have added an weak alias to sys/rump/librump/rumpkern/emul.c to handle the case where librumpvfs is not present. Please revert and fix correctly. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/sys/arch/sparc64/sparc64
This breaks the build of usr.sbin/crash: /work/build/src/usr.sbin/crash/../../sys/arch/sparc64/sparc64/db_trace.c: In function 'db_stack_trace_print': /work/build/src/usr.sbin/crash/../../sys/arch/sparc64/sparc64/db_trace.c:166:37: error: 'VM_MAX_KERNEL_ADDRESS' undeclared (first use in this function); did you mean 'VM_MAXADDRESS'? if (frame < KERNBASE || frame >= VM_MAX_KERNEL_ADDRESS) ^ VM_MAXADDRESS /work/build/src/usr.sbin/crash/../../sys/arch/sparc64/sparc64/db_trace.c:166:37: note: each undeclared identifier is reported only once for each function it appears in -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig > On 22. May 2019, at 09:40, Martin Husemann wrote: > > Module Name: src > Committed By: martin > Date: Wed May 22 07:40:09 UTC 2019 > > Modified Files: > src/sys/arch/sparc64/sparc64: db_trace.c > > Log Message: > Fix previous and use the original patch from PR port-sparc64/54221 > instead (XXX should fix comments in param.h) > > > To generate a diff of this commit: > cvs rdiff -u -r1.51 -r1.52 src/sys/arch/sparc64/sparc64/db_trace.c > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. > signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/tools
> On 7. May 2019, at 12:17, matthew green wrote: > >> Diff is NOT reversed. > > ah yes, i see. please commit. Done. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/tools
> On 7. May 2019, at 11:54, matthew green wrote:
>
> reversed patch, but i think you don't need the extra .WAIT step.
>
> it worked for me just before ${TOOLCHAIN_BITS} but you can probably
> put it anywhere in that set.
>
> please commit!
Diff is NOT reversed. My Rev. 1.202 makefile has:
SUBDIR+= grep xz-lib libprop
.if ${TOOLS_BUILDRUMP} == "no"
SUBDIR += .WAIT texinfo \
.WAIT tic \
.WAIT pax \
.WAIT ${TOOLCHAIN_BITS} \
Here libprop needs pax, but pax gets built after it.
--
J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/tools
> On 7. May 2019, at 06:29, Jason R Thorpe wrote: > > Module Name: src > Committed By: thorpej > Date: Tue May 7 04:29:45 UTC 2019 > > Modified Files: > src/tools: Makefile > Added Files: > src/tools/libprop: Makefile > > Log Message: > Add support for libprop as a host tool library. > > > To generate a diff of this commit: > cvs rdiff -u -r1.201 -r1.202 src/tools/Makefile > cvs rdiff -u -r0 -r1.1 src/tools/libprop/Makefile Looks like libprop needs pax. Is the attached diff ok? -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany) Makefile.diff Description: Binary data
Re: CVS commit: src/sys
> On 19. Apr 2019, at 03:52, Ryota Ozaki wrote: > > Module Name: src > Committed By: ozaki-r > Date: Fri Apr 19 01:52:56 UTC 2019 > > Modified Files: > src/sys/kern: kern_lwp.c kern_softint.c subr_psref.c > src/sys/rump/kern/lib/libsysproxy: sysproxy.c > src/sys/sys: lwp.h userret.h > > Log Message: > Implement a simple psref leak detector > > It detects leaks by counting up the number of held psref by an LWP and > checking > its zeroness at the end of syscalls and softint handlers. For the counter, a > unused field of struct lwp is reused. For DIAGNOSTIC-only operations LWP specific data (see kern/subr_lwp_specificdata.c) is a better choice. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/sys/kern
> On 27. Sep 2018, at 09:44, Manuel Bouyer wrote: > > On Thu, Sep 27, 2018 at 01:03:40AM +, Emmanuel Dreyfus wrote: >> Module Name: src >> Committed By:manu >> Date:Thu Sep 27 01:03:40 UTC 2018 >> >> Modified Files: >> src/sys/kern: vfs_trans.c >> >> Log Message: >> Work around deadlock between fstchg and fstcnt >> >> When suspending a filesystem in fstrans_setstate(), we wait on >> fstcnt for threads to finish transactions. While we do this, any >> thread trying to start a filesystem transaction will wait on fstchg >> in fstrans_start(), a situation which can deadlock. >> >> The wait for fstcnt in fstrans_setstate() can be interrupted by >> a signal, but the wait for fstchg in fstrans_start() cannot. Once >> most processes are stuck in fstchg, it is impossible to send a >> signal to the thread that waits on fstcnt, because no process >> respond anymore to user input. >> >> We fix that by adding a timeout to the wait on fstcnt in >> fstrans_setstate(). This means suspending a filesystem may fail, >> but it was already the case when the sleep was interupted by >> a signal, hence calling function must already handle a possible >> failure. > > Actually callers do not, they just forward the failure. > This means that things like e.g. umount or snapshots will randomly > fail (whenever fstrans_setstate() can actually take a long > time without deadlock has to be determined, but I suspect it can). > > I think fstrans_setstate() should pause and retry in this case. As Manuel said ... This change is wrong, there is no upper time limit for a successfull file system suspension. Please revert ASAP. I will take a look into this PR next week. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/usr.bin/kdump
> On 2. Jun 2018, at 22:07, matthew green wrote:
>
> Module Name: src
> Committed By: mrg
> Date: Sat Jun 2 20:07:15 UTC 2018
>
> Modified Files:
> src/usr.bin/kdump: mkioctls
>
> Log Message:
> just include for mkioctls. this works fine for me
> for several platforms and fixes the clang build.
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.50 -r1.51 src/usr.bin/kdump/mkioctls
This breaks the build:
In file included from /build/devel/src/sys/net/if.h:99:0,
from kdump-ioctl.c:23:
/build/devel/src/sys/altq/if_altq.h:48:2: error: unknown type name 'kmutex_t'
kmutex_t *ifq_lock;
^~~~
The real problem seems the sys include search path:
#include <...> search starts here:
/build/devel/obj/dist.amd64/usr/X11R7/include/libdrm
/build/devel/obj/dist.amd64/usr/X11R7/include/pixman-1
/build/devel/obj/dist.amd64/usr/X11R7/include
/build/devel/src/external/cddl/osnet/sys
/build/devel/src/external/cddl/osnet/dist/uts/common
/build/devel/src/usr.bin/ktrace
/build/devel/src/sys
/build/devel/obj/dist.amd64/usr/include/gcc-6
/build/devel/obj/tools.amd64/lib/gcc/x86_64--netbsd/6.4.0/include-fixed
/build/devel/obj/dist.amd64/usr/include
Most includes come from "cddl/osnet/sys/sys" which looks wrong.
Suppose the search better starts with "-I${NETBSDSRCDIR}/sys" ...
--
J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germa
Re: CVS commit: src/usr.bin/make
> On 12. May 2018, at 20:17, Simon J. Gerraty <s...@netbsd.org> wrote: > > Module Name: src > Committed By: sjg > Date: Sat May 12 18:17:04 UTC 2018 > > Modified Files: > src/usr.bin/make: job.c > > Log Message: > Skip setting wantToken. > > polling the job token pipe adds a lot of overhead > for little gain. > For now, just leave wantToken=0 > > And avoid busy waiting when no tokens are available and > no jobs are running. > > Reviewed by: christos > > > To generate a diff of this commit: > cvs rdiff -u -r1.192 -r1.193 src/usr.bin/make/job.c > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. > After this commit parallel builds take much longer. Building amd64 release with -j16 for example goes from 45 to 380 minutes. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src
> On 29. Apr 2018, at 21:47, Sevan Janiyan <se...@netbsd.org> wrote: > > Module Name: src > Committed By: sevan > Date: Sun Apr 29 19:47:35 UTC 2018 > > Modified Files: > src/crypto/external/bsd/netpgp: Makefile > src/distrib/sets/lists/base: shl.mi > Added Files: > src/crypto/external/bsd/netpgp/bindings: Makefile > src/crypto/external/bsd/netpgp/bindings/lua: Makefile > > Log Message: > Hello netpgp(3lua) This breaks at least amd64: checkflist ===> distrib/sets == 1 missing files in DESTDIR Files in flist but missing from DESTDIR. File wasn't installed ? -- ./usr/lib/i386/lua/5.3/netpgp.so ==== end of 1 missing files == -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/kern
> On 17. Apr 2018, at 03:38, Ryota Ozaki <ozak...@netbsd.org> wrote: > > On Tue, Apr 17, 2018 at 5:25 AM Juergen Hannken-Illjes <hann...@netbsd.org> > wrote: > >> Module Name:src >> Committed By: hannken >> Date: Mon Apr 16 20:25:21 UTC 2018 > >> Modified Files: >> src/sys/kern: subr_pserialize.c > >> Log Message: >> Function pserialize_perform() usually succeeds after two cross calls >> so defer kpause() to iterations three and above. > >> Speeds up VOP_REVOKE() on /proc/XXX/status by a factor of ~12. > >> Ok: core@ > > Any chance to pull up it to -8? Ticket #771 -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/miscfs/procfs
> On 7. Apr 2018, at 21:27, Christos Zoulas <chris...@astron.com> wrote: > > In article <20180407134242.bceb9f...@cvs.netbsd.org>, > Juergen Hannken-Illjes <source-changes-d@NetBSD.org> wrote: >> -=-=-=-=-=- >> >> Module Name: src >> Committed By:hannken >> Date:Sat Apr 7 13:42:42 UTC 2018 >> >> Modified Files: >> src/sys/miscfs/procfs: procfs_vnops.c >> >> Log Message: >> Lock the target cwdi and take an additional reference to the >> vnode we are interested in to prevent it from disappearing >> before getcwd_common(). >> >> Should fix PR kern/53096 (netbsd-8 crash on heavy disk I/O) > > Pullup? Already requested with ticket #702. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys
> On 27. Oct 2017, at 11:59, Utkarsh Anand <utkarsh...@netbsd.org> wrote: > > Module Name: src > Committed By: utkarsh009 > Date: Fri Oct 27 09:59:17 UTC 2017 > > Modified Files: > src/sys/arch/x86/x86: intr.c > src/sys/ddb: db_interface.h db_panic.c > src/sys/kern: init_main.c subr_autoconf.c subr_disk.c subr_prf.c > vfs_subr.c vfs_wapbl.c > src/sys/sys: systm.h > src/sys/ufs/ufs: ufs_lookup.c > > Log Message: > [syzkaller] Attempted fix for https://github.com/google/syzkaller/issues/399 > > syzkaller was failing to extract constants because of the above mentioned > issue so I had to redeclare printf in sys/sys/systm.h > For more information on syzkaller, visit: https://github.com/google/syzkaller This looks wrong: kernel printf != userland printf If we were to go this route the return value should match userland, either -1 on error or number of characters printed. The other kernel printf variants should be changed too. It needs a kernel version bump. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src
> On 08 Dec 2016, at 15:41, Nathanial Sloss <n...@netbsd.org> wrote: > > On Thu, 8 Dec 2016 23:57:51 J. Hannken-Illjes wrote: >> +#defineD_MCLOSE0x0004 >> >> What is a MCLOSE device class? >> > > Normally character devices are don't execute the close function untill the > last instance of device is closed. MCLOSE is short for multiple close, for > use when you want the close function to run when any instance of the device, > in this case audio, is closed. So it should become a flag like 0x0400, not a class in the 0x00ff range. Where was this change discussed? -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src
> On 08 Dec 2016, at 11:28, Nathanial Sloss <n...@netbsd.org> wrote: > > Module Name: src > Committed By: nat > Date: Thu Dec 8 10:28:44 UTC 2016 > > Modified Files: > src/sys/dev: audio.c audiovar.h files.audio > src/sys/miscfs/specfs: spec_vnops.c > src/sys/sys: audioio.h conf.h > src/usr.bin/audio/ctl: audioctl.1 ctl.c > > Log Message: > The audio sub-system now supports the following features as > posted to tech-kern: > >* Simultaneous playback and mixing of multiple streams >* Playback streams can be of different encoding, frequency, precision > and number of channels >* Simultaneous recording to different formats >* One audio device per process >* Sysctls to set the common format frequency, precision and channels >* Independent mixer controls for recording/playback per stream >* Utilizes little cpu time for multiple streams / good performance >* Compatible with existing programs that use OSS/NetBSD audio >* Changes to audioctl(1) to allow specifying process id for corresponding > audio device > > > To generate a diff of this commit: > cvs rdiff -u -r1.268 -r1.269 src/sys/dev/audio.c > cvs rdiff -u -r1.46 -r1.47 src/sys/dev/audiovar.h > cvs rdiff -u -r1.3 -r1.4 src/sys/dev/files.audio > cvs rdiff -u -r1.165 -r1.166 src/sys/miscfs/specfs/spec_vnops.c +#defineD_MCLOSE0x0004 What is a MCLOSE device class? > cvs rdiff -u -r1.34 -r1.35 src/sys/sys/audioio.h > cvs rdiff -u -r1.146 -r1.147 src/sys/sys/conf.h > cvs rdiff -u -r1.19 -r1.20 src/usr.bin/audio/ctl/audioctl.1 > cvs rdiff -u -r1.40 -r1.41 src/usr.bin/audio/ctl/ctl.c > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. > -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys
> On 11 Nov 2016, at 22:28, Jaromír Doleček <jaromir.dole...@gmail.com> wrote:
>
> 2016-11-11 11:52 GMT+01:00 J. Hannken-Illjes <hann...@eis.cs.tu-bs.de>:
>> Returning a pointer to an arbitrary list element and using it
>> later is bad design. Would be better to define as:
>>
>> wapbl_unregister_deallocation(struct wapbl *wl, daddr_t blk, int len)
>> {
>
> I simply want to have it O(1). I think it is useful to keep the error
> path there fast, as it would be quite common for wapbl case. Also just
> passing the (opaque) pointer makes it simpler.
This error path (call UFS_WAPBL_UNREGISTER_DEALLOCATION()) is not that
common and runs not more than once during one run of ffs_truncate().
I don't see a need to have it O(1).
--
J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys
> On 07 Nov 2016, at 14:04, Christos Zoulas <chris...@astron.com> wrote: > > In article > <camnsw54_di9cbtc6ckmlnan6j--uljkhvswvne5q2y+u3c6...@mail.gmail.com>, > JaromĆr DoleÄ ek <jaromir.dole...@gmail.com> wrote: >> >> Anyway, IMO eventual change to allow fsck to DTRT after crash in >> ffs_truncate() should go separately from the crash fix. > > While fsck could be fixed, IMO, it is best for the kernel to try to leave > things as consistent as possible avoid scary noise from fsck and to keep > recovery simpler. It is impossible to fix fsck here: fsck gets a file system where some inodes have the same block allocated. It is impossible to determine which allocation is wrong as its inode was in truncation. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys
> On 07 Nov 2016, at 21:53, Jaromír Doleček <jaromir.dole...@gmail.com> wrote: > > 2016-11-07 12:11 GMT+01:00 J. Hannken-Illjes <hann...@eis.cs.tu-bs.de>: >> It gets used when a block of block pointers has been deallocated and >> brelsed, that is NOT written to disk. If we allow the deallocation of >> this block to fail and ufs_truncate_retry() runs ffs_truncate again >> it will read the block from disk and free already freed blocks. > > Good point. The same is however true for ALL the calls in the chain, > so then we should have to really use FORCE everywhere. No. If one of the other calls (clearing one pointer in a block) fails we already bwrite() (if (error != 0 ...). In the wapbl case writing all blocks would result in transitions too big for the log. I tried it ... > One solution is to always bwrite() or bdwrite() back even fully zeroed > block for wapbl case instead of brelse(BC_INVAL). I think that for > fsck to reliably recover from crash within truncate, this might > actually be needed also for !wapbl case. No. The !wapbl case is safe. The block is either written after the copy (before we change it) or from ffs_truncate() when it sets "sync = 1". For wapbl case see above. > Another way how to solve this would be to try to register the > deallocation and on error bail out, before the diving. It would > require cancelling the registration if the diving call returns EAGAIN > however. > >> You mean this: >> >>error = ffs_indirtrunc(ip, nlbn, FFS_FSBTODB(fs, nb), >>(daddr_t)-1, level - 1, countp); >> - if (error) >> - goto out; >> + if (error == EAGAIN) >> + goto out; >> + else if (error && !allerror) >> + allerror = error; > > No, I mean the copy logic and big blocks with condition on > ip->i_ump->um_mountp->mnt_wapbl. The copy logic is here to prevent corruption beyond fsck capabilities and therefore we need in the !wapbl case. There IS a reason it was here since the beginning of time. Not sure if it would make sense to split it into ffs_indir_trunc() and ffs_indir_trunc_wapbl(). Possibly easier to understand but prone to errors fixed in only one of them. >> I don't understand ... do you want to split into two diffs? > > I prefer to split commits into incremental changes if reasonably > possible, so it's easier to review and revise. That's all. That's why > I prefer the fix for immediate corruption to go separately. My patch contains corruption issues only and it passes ATF and it passes my stress test which is a bit more than just some fsx. As -current currently corrupts file systems we should either fix it very soon or revert your changes completely. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys
> On 07 Nov 2016, at 11:54, Jaromír Doleček <jaromir.dole...@gmail.com> wrote: > > 2016-11-07 10:25 GMT+01:00 J. Hannken-Illjes <hann...@eis.cs.tu-bs.de>: >> The first part should not be necessary. After the loop we should have >> "i == last" -- from a quick look "i < last" is impossible. > > Yes, I know - it's just to make the diff vs 1.117 smaller and hence > easier to review. I want to commit this separately. > >> The second part is right and responsible for most of the panics. > > Right. > >> Your patch still doesn't address my second observation, if the >> machine crashs inside ffs_truncate we leave the file system in a >> state fsck can't handle. The blocks we freed get attached to other >> nodes before they get cleared from our on-disk copy leading to >> duplicate blocks. > > That patch is not really very ideal. > > One is UFS_WAPBL_REGISTER_DEALLOCATION_FORCE(), which should only be > used as desperate measure. It gets used when a block of block pointers has been deallocated and brelsed, that is NOT written to disk. If we allow the deallocation of this block to fail and ufs_truncate_retry() runs ffs_truncate again it will read the block from disk and free already freed blocks. > Second is that the patch adds IMO too much difference for wapbl and > !wapbl case, to already quite complicated function. Also it's slightly > confusing that the !wapbl change now doesn't support failure in the > middle any more - it will just leave the block inconsistent. I need to > think a little about this, maybe there is some better way. The wapbl and !wapbl cases ARE different. You mean this: error = ffs_indirtrunc(ip, nlbn, FFS_FSBTODB(fs, nb), (daddr_t)-1, level - 1, countp); - if (error) - goto out; + if (error == EAGAIN) + goto out; + else if (error && !allerror) + allerror = error; > Anyway, IMO eventual change to allow fsck to DTRT after crash in > ffs_truncate() should go separately from the crash fix. I don't understand ... do you want to split into two diffs? -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys
> On 07 Nov 2016, at 00:11, Jaromír Doleček <jaromir.dole...@gmail.com> wrote:
>
>> On Fri, Nov 04, 2016 at 04:44:10PM +0100, J. Hannken-Illjes wrote:
>>> - This change results in "panic: ffs_blkfree_common: freeing free block"
>>> if I put a file system under stress (*1).
>>>
>>> - I suppose not zeroing the blocks to be freed before freeing them
>>> makes the life of fsck harder.
>>>
>>> - Running "brelse(bp, BC_INVAL)" doesn't look OK.
>
> The brelse(bp, BC_INVAL) call was there before as well, but the
> condition changed and is wrong.
>
> I can repeat the problem with your script and the packaged fsx (thanks
> Thomas). Whipped up a patch to what looked wrong there, and it no
> longer panics for me. Patch is attached. I'll test further and commit
> it tomorrow.
Index: ffs_inode.c
===
RCS file: /cvsroot/src/sys/ufs/ffs/ffs_inode.c,v
retrieving revision 1.118
diff -u -p -r1.118 ffs_inode.c
--- ffs_inode.c 28 Oct 2016 20:38:12 - 1.118
+++ ffs_inode.c 6 Nov 2016 23:09:15 -
@@ -675,18 +675,18 @@ ffs_indirtrunc(struct inode *ip, daddr_t
* Recursively free blocks on the now last partial indirect block.
*/
if (level > SINGLE && lastbn >= 0) {
- nb = RBAP(ip, last);
+ last = lastbn % factor;
+ nb = RBAP(ip, i);
if (nb != 0) {
error = ffs_indirtrunc(ip, nlbn, FFS_FSBTODB(fs, nb),
- lastbn % factor, level - 1,
- countp);
+ last, level - 1, countp);
if (error)
goto out;
}
}
out:
- if (RBAP(ip, 0) == 0) {
+ if (lastbn < 0 && error == 0) {
/* all freed, release without writing back */
brelse(bp, BC_INVAL);
} else {
The first part should not be necessary. After the loop we should have
"i == last" -- from a quick look "i < last" is impossible.
The second part is right and responsible for most of the panics.
Your patch still doesn't address my second observation, if the
machine crashs inside ffs_truncate we leave the file system in a
state fsck can't handle. The blocks we freed get attached to other
nodes before they get cleared from our on-disk copy leading to
duplicate blocks.
The attached diff:
- Brings back the non-wapbl behaviour to first clear the allocations
in the on-disk node and then deallocating them.
- Fixes the condition to brelse() on exit like your diff does.
- Makes sure a block that is completely deallocated and NOT written
to disk gets deallocated even if this deallocation would fail.
Please review.
--
J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
ffs_inode.c.diff
Description: Binary data
Re: CVS commit: src/sys
> On 28 Oct 2016, at 22:38, Jaromir Dolecek <jdole...@netbsd.org> wrote: > > Module Name: src > Committed By: jdolecek > Date: Fri Oct 28 20:38:12 UTC 2016 > > Modified Files: > src/sys/kern: vfs_wapbl.c > src/sys/sys: wapbl.h > src/sys/ufs/ffs: ffs_alloc.c ffs_inode.c ffs_snapshot.c > src/sys/ufs/ufs: ufs_extern.h ufs_inode.c ufs_rename.c ufs_vnops.c > ufs_wapbl.h > > Log Message: > reorganize ffs_truncate()/ffs_indirtrunc() to be able to partially > succeed; change wapbl_register_deallocation() to return EAGAIN > rather than panic when code hits the limit > > callers changed to either loop calling ffs_truncate() using new > utility ufs_truncate_retry() if their semantics requires it, or > just ignore the failure; remove ufs_wapbl_truncate() > > this fixes possible user-triggerable panic during truncate, and > resolves WAPBL performance issue with truncates of large files > > PR kern/47146 and kern/49175 - This change results in "panic: ffs_blkfree_common: freeing free block" if I put a file system under stress (*1). - I suppose not zeroing the blocks to be freed before freeing them makes the life of fsck harder. - Running "brelse(bp, BC_INVAL)" doesn't look OK. Please fix or revert soon. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany) *1 The stress is running the shell script attached. I usually get a panic within less than a minute on a 16-core virtual amd64. T.sh Description: Binary data fsx.c Description: Binary data
Re: CVS commit: src/sys
> On 06 Jul 2016, at 10:42, Ryota Ozaki <ozak...@netbsd.org> wrote:
>
> Module Name: src
> Committed By: ozaki-r
> Date: Wed Jul 6 08:42:34 UTC 2016
>
> Modified Files:
> src/sys/net: if_stf.c
> src/sys/netinet: in.c in_gif.c in_pcb.c in_var.h ip_carp.c ip_icmp.c
> ip_input.c
> src/sys/netipsec: key.c
>
> Log Message:
> Switch the IPv4 address list to pslist(9)
>
> Note that we leave the old list just in case; it seems there are some
> kvm(3) users accessing the list. We can remove it later if we confirmed
> nobody does actually.
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.93 -r1.94 src/sys/net/if_stf.c
> cvs rdiff -u -r1.170 -r1.171 src/sys/netinet/in.c
@@ -639,8 +644,10 @@ in_control(struct socket *so, u_long cmd
if (newifaddr) {
TAILQ_INSERT_TAIL(_ifaddrhead, ia, ia_list);
ifaref(>ia_ifa);
ifa_insert(ifp, >ia_ifa);
+ TAILQ_INSERT_TAIL(_ifaddrhead, ia, ia_list);
+ IN_ADDRLIST_WRITER_INSERT_TAIL(ia);
LIST_INSERT_HEAD(_IFADDR_HASH(ia->ia_addr.sin_addr.s_addr),
ia, ia_hash);
IN_ADDRHASH_WRITER_INSERT_HEAD(ia);
} else if (need_reinsert) {
This one looks wrong -- we enqueue "ia" twice.
--
J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys
> On 26 May 2016, at 13:17, Paul Goyette <p...@whooppee.com> wrote: > > On Thu, 26 May 2016, Juergen Hannken-Illjes wrote: > >> Module Name: src >> Committed By:hannken >> Date:Thu May 26 11:09:55 UTC 2016 >> >> Modified Files: >> src/sys/kern: vfs_vnode.c >> src/sys/sys: vnode.h >> >> Log Message: >> Use vnode state to replace VI_MARKER, VI_CHANGING, VI_XLOCK and VI_CLEAN. >> >> Presented on tech-kern@ > > Do we need a kernel version bump? No -- the flags removed were already protected by _VFS_VNODE_PRIVATE and therefore invisible outside sys/kern/vfs_*. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/arch/sparc
> On 05 Apr 2016, at 15:05, Martin Husemann <mar...@duskware.de> wrote:
>
> On Tue, Apr 05, 2016 at 03:02:25PM +0200, J. Hannken-Illjes wrote:
>> The attached hack works for me -- looks like we cannot use a mutex
>> so early ...
>
> Or just skip the mutex_{enter,exit} while cold?
The first crash is the result of mutex_init -> lockdebug_alloc().
--
J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/arch/sparc
> On 01 Apr 2016, at 22:21, Palle Lyckegaard <pa...@netbsd.org> wrote: > > Module Name: src > Committed By: palle > Date: Fri Apr 1 20:21:45 UTC 2016 > > Modified Files: > src/sys/arch/sparc/include: openfirm.h > src/sys/arch/sparc/sparc: openfirm.c promlib.c > > Log Message: > sun4v: Workaround for OpenBoot feature where a 64-bit address is truncated to > a 32-bit address. This happends when a write to the console > (/virtual-devices@100/console@1) is done. Avoid this by using a static buffer > that is mapped below 4GB. Thanks to Tarl Neustaedter for explaining how > OpenBoot works. ok martin@ > > > To generate a diff of this commit: > cvs rdiff -u -r1.9 -r1.10 src/sys/arch/sparc/include/openfirm.h > cvs rdiff -u -r1.20 -r1.21 src/sys/arch/sparc/sparc/openfirm.c > cvs rdiff -u -r1.44 -r1.45 src/sys/arch/sparc/sparc/promlib.c > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. > This breaks (at least) LOCKDEBUG kernel on Sun V240: Loading netbsd: 10104048+639024+609968 [638184+418249]=0xe33790 ERROR: Last Trap: Fast Data Access MMU Miss The attached hack works for me -- looks like we cannot use a mutex so early ... -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany) openfirm.diff Description: Binary data
Re: CVS commit: src/external/gpl3/gcc.old/dist/gcc
> On 08 Mar 2016, at 04:58, Christos Zoulas <chris...@netbsd.org> wrote: > > Module Name: src > Committed By: christos > Date: Tue Mar 8 03:58:08 UTC 2016 > > Modified Files: > src/external/gpl3/gcc.old/dist/gcc: output.h varasm.c > src/external/gpl3/gcc.old/dist/gcc/config/i386: i386.c > > Log Message: > Fix copy relocations against protected symbols from: >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65248 > > > To generate a diff of this commit: > cvs rdiff -u -r1.3 -r1.4 src/external/gpl3/gcc.old/dist/gcc/output.h \ >src/external/gpl3/gcc.old/dist/gcc/varasm.c > cvs rdiff -u -r1.3 -r1.4 \ >src/external/gpl3/gcc.old/dist/gcc/config/i386/i386.c This change seems to break rump, number of failed tests on arch amd64 goes from 19 to 1342, same problem on i386. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys
On 27 Aug 2015, at 13:26, Michael van Elst mlel...@serpens.de wrote:
On Thu, Aug 27, 2015 at 11:48:15AM +0200, J. Hannken-Illjes wrote:
disk_busy(dksc-sc_dkdev);
+ mutex_exit(dksc-sc_iolock);
error = dkd-d_diskstart(dksc-sc_dev, bp);
+ mutex_enter(dksc-sc_iolock);
if (error == EAGAIN) {
+ dksc-sc_deferred = bp;
disk_unbusy(dksc-sc_dkdev, 0, (bp-b_flags
B_READ));
break;
}
Looks racy: what if two threads run dk_strategy() - dk_start() and
both get EAGAIN. Will it leak a buffer when the second thread tries
to save bp and dksc-sc_deferred already holds the buffer from the
first thread?
Looks like it. sc_deferred probably needs to become a fcfs bufq.
Currently this could happen for the ld driver (others are not MP_SAFE).
It would be simpler if you could unget something into the original
bufq.
This was the intention with bufq_peek. All current bufq strategies
seem to keep the current buf the same over bufq_peek .. bufq_get
even if more work comes in via bufq_put.
I'm quite sure most of our floppy drivers work this way.
It is just callers of bufq_drain that have to protect against
bufq_pee .. bufq_get sequences.
--
J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys
On 27 Aug 2015, at 13:26, Michael van Elst mlel...@serpens.de wrote:
On Thu, Aug 27, 2015 at 11:48:15AM +0200, J. Hannken-Illjes wrote:
disk_busy(dksc-sc_dkdev);
+ mutex_exit(dksc-sc_iolock);
error = dkd-d_diskstart(dksc-sc_dev, bp);
+ mutex_enter(dksc-sc_iolock);
if (error == EAGAIN) {
+ dksc-sc_deferred = bp;
disk_unbusy(dksc-sc_dkdev, 0, (bp-b_flags
B_READ));
break;
}
Looks racy: what if two threads run dk_strategy() - dk_start() and
both get EAGAIN. Will it leak a buffer when the second thread tries
to save bp and dksc-sc_deferred already holds the buffer from the
first thread?
Looks like it. sc_deferred probably needs to become a fcfs bufq.
Currently this could happen for the ld driver (others are not MP_SAFE).
It would be simpler if you could unget something into the original
bufq.
Another approach is to enqueue the buffer from dk_strategy(), use
dk_start() without bp to notify the device. The device start
routine would take buffers from the queue and start them until
it gets out of resources and the device would call its start
routine on completion of a request.
--
J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys
On 27 Aug 2015, at 07:51, Michael van Elst mlel...@netbsd.org wrote:
Module Name: src
Committed By: mlelstv
Date: Thu Aug 27 05:51:50 UTC 2015
Modified Files:
src/sys/arch/xen/xen: xbd_xenbus.c
src/sys/dev: cgd.c dksubr.c dkvar.h ld.c
Log Message:
Make dksubr use a spin-mutex again, since some drivers still call dk_done
from hardware interrupt. Instead, release mutex while calling start routine.
The buffer peek/use/get sequence which can no longer be atomic. So consume
the buffer directly and on error privately save and retry the buffer later.
The dk_drain function is used to flush such a deferred buffer together with
the buffer queue.
Adjust drivers to use dk_drain.
Fix an error path where dk_done was called while the lock was already held.
To generate a diff of this commit:
cvs rdiff -u -r1.72 -r1.73 src/sys/arch/xen/xen/xbd_xenbus.c
cvs rdiff -u -r1.103 -r1.104 src/sys/dev/cgd.c
cvs rdiff -u -r1.73 -r1.74 src/sys/dev/dksubr.c
disk_busy(dksc-sc_dkdev);
+ mutex_exit(dksc-sc_iolock);
error = dkd-d_diskstart(dksc-sc_dev, bp);
+ mutex_enter(dksc-sc_iolock);
if (error == EAGAIN) {
+ dksc-sc_deferred = bp;
disk_unbusy(dksc-sc_dkdev, 0, (bp-b_flags B_READ));
break;
}
Looks racy: what if two threads run dk_strategy() - dk_start() and
both get EAGAIN. Will it leak a buffer when the second thread tries
to save bp and dksc-sc_deferred already holds the buffer from the
first thread?
cvs rdiff -u -r1.21 -r1.22 src/sys/dev/dkvar.h
cvs rdiff -u -r1.91 -r1.92 src/sys/dev/ld.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
--
J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/dev/ic
Sure, will request pullups after one week (Jul 29). -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany) On 27 Jul 2015, at 02:15, Erik Fair f...@netbsd.org wrote: Pull up to netbsd-7 and netbsd-6? Erik On Jul 22, 2015, at 01:33, Juergen Hannken-Illjes hann...@netbsd.org wrote: Module Name: src Committed By:hannken Date:Wed Jul 22 08:33:51 UTC 2015 Modified Files: src/sys/dev/ic: mpt_netbsd.c Log Message: Adapter leaks requests when mpt_event_notify_reply() has to acknowledge an event leading to adapter resource shortage messages when the scsipi subsystem tries to use all adapt_openings. Change mpt_ctlop() to free the request on event MPI_FUNCTION_EVENT_ACK. Tested on a SunFire X4275 with Symbios Logic SAS1068E (1000:0058, rev. 4). To generate a diff of this commit: cvs rdiff -u -r1.31 -r1.32 src/sys/dev/ic/mpt_netbsd.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Re: CVS commit: src/sys/miscfs
On 01 Jul 2015, at 08:19, Ryota Ozaki ozak...@netbsd.org wrote: On Tue, Jun 30, 2015 at 3:19 PM, Juergen Hannken-Illjes hann...@netbsd.org wrote: Module Name:src Committed By: hannken Date: Tue Jun 30 06:19:22 UTC 2015 Modified Files: src/sys/miscfs/deadfs: dead_vfsops.c src/sys/miscfs/specfs: spec_vnops.c Log Message: Redo previous again, v_specnode is invariant but not unique. Set vp-v_data = vp and use v_data as key. The change seems to break ATF tests: http://releng.netbsd.org/b5reports/i386/commits-2015.06.html#end The error message is: large_blk: panic: kernel diagnostic assertion devvp-v_data == NULL failed: file /tmp/bracket/build/2015.06.30.06.42.06-i386/src/sys/ufs/mfs/mfs_vfsops.c, line 324 Should be fixed now. Passes ATF on i386. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/miscfs/specfs
On 29 Jun 2015, at 18:47, David Holland dholland-sourcechan...@netbsd.org wrote: On Mon, Jun 29, 2015 at 12:25:49PM -0400, Christos Zoulas wrote: Module Name: src Committed By:christos Date:Mon Jun 29 16:25:49 UTC 2015 Modified Files: src/sys/miscfs/specfs: spec_vnops.c Log Message: Revert previous, and explain why. Hrm, shouldn't it be vp? (as it is, it's using the uvm_object lock pointer as the key) Yes, just changed the key to the address of vp-v_specdev which is invariant over the lifetime of the vnode. Coverity is quite cool ... -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src
On 02 Jun 2015, at 08:24, Martin Husemann mar...@duskware.de wrote: On Mon, Jun 01, 2015 at 09:44:58PM +0200, Emmanuel Dreyfus wrote: Do you have both two fixes from Christos? Yes, that test was run with -current as of yesterday morning: Build locally, cvs.netbsd.org source tree date: 2015-06-01 08:50 UTC The official atf runs for i386, amd64 and sparc show sporadic failures for this test. Does it fail on every run? -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src
On 29 May 2015, at 16:26, J. Hannken-Illjes hann...@eis.cs.tu-bs.de wrote: On 29 May 2015, at 09:37, Emmanuel Dreyfus m...@netbsd.org wrote: Module Name: src Committed By:manu Date:Fri May 29 07:37:32 UTC 2015 Modified Files: src/include: limits.h src/lib/libpthread: pthread.c pthread_int.h pthread_key_create.3 pthread_tsd.c src/lib/libpthread_dbg: pthread_dbg.c Log Message: Make PTHREAD_KEYS_MAX dynamically adjustable snip With this change some programs fail with assertion pthread__tsd_destructors[key] != NULL failed file src/lib/libpthread/pthread_tsd.c, line 169, function pthread__add_specific Machine is amd64, 16 cores, KVM host. Among others gunzip fails. Reverting this commit removes the failure. With christos fix dated Fri May 29 16:05:13 UTC 2015 it works for me. Thanks, -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src
On 29 May 2015, at 16:57, Emmanuel Dreyfus m...@netbsd.org wrote: On Fri, May 29, 2015 at 04:26:33PM +0200, J. Hannken-Illjes wrote: With this change some programs fail with assertion pthread__tsd_destructors[key] != NULL failed file src/lib/libpthread/pthread_tsd.c, line 169, function pthread__add_specific Machine is amd64, 16 cores, KVM host. Among others gunzip fails. Reverting this commit removes the failure. I have seen that with older libc: setting breackpoints on pthread_key_create() and pthread_setspecific() shows that malloc() calls the later without calling the former, which is a bug. It worked before previously, pthread__tsd_destructors[] was not zero'ed and contained random data. I tested filling the array with non NULL random data and the assertion is not fired anymore. More recent libc did not seem to have the problem. How old is yours? May 29 11:50 /lib/libc.so.12.197 from after your commit. Can you confirm the pthread_key_create()/pthread_setspecific() test? As I can't install (no gunzip ...) I can't run tests. Reverting your commit and adding the attached patch I get: gunzip /sets/kern-work.tgz /dev/null [ pthread__init ] [ pthread_key_create : 0 / 0x7f7ff640759c ] [ pthread__add_specific : 0 / 0x7f7ff640759c ] Debug patch: --- pthread.c 16 Dec 2014 20:05:54 - 1.145 +++ pthread.c 29 May 2015 16:11:34 - @@ -233,4 +233,5 @@ pthread__init(void) pthread_atfork(NULL, NULL, pthread__fork_callback); __isthreaded = 1; +fprintf(stderr, [ %s ]\n, __func__); } --- pthread_tsd.c 21 Mar 2013 16:49:12 - 1.11 +++ pthread_tsd.c 29 May 2015 16:11:34 - @@ -35,4 +35,5 @@ __RCSID($NetBSD: pthread_tsd.c,v 1.11 2 /* Functions and structures dealing with thread-specific data */ #include errno.h +#include stdio.h #include pthread.h @@ -92,4 +93,5 @@ pthread_key_create(pthread_key_t *key, v * to be found. */ +fprintf(stderr, [ %s - EAGAIN ]\n, __func__); pthread_mutex_unlock(tsd_mutex); return EAGAIN; @@ -100,4 +102,5 @@ pthread_key_create(pthread_key_t *key, v pthread__assert(PTQ_EMPTY(pthread__tsd_list[i])); pthread__tsd_destructors[i] = destructor ? destructor : null_destructor; +fprintf(stderr, [ %s : %d / %p ]\n, __func__, i, pthread__tsd_destructors[i]); nextkey = (i + 1) % PTHREAD_KEYS_MAX; @@ -134,4 +137,6 @@ pthread__add_specific(pthread_t self, pt pthread_mutex_lock(tsd_mutex); +fprintf(stderr, [ %s : %d / %p ]\n, __func__, key, +pthread__tsd_destructors[key]); pthread__assert(pthread__tsd_destructors[key] != NULL); pt = self-pt_specific[key]; -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src
Jörg, seit diesem Commit gibt es keine i386 Builds mehr: src/tests/include/t_inttypes.c:173:2: error: format '%hhd' expects argument of type 'signed char *', but argument 3 has type 'int_fast8_t *' [-Werror=format=] SCAN(SCNdFAST8, if8); ^ src/tests/include/t_inttypes.c:173:2: error: format '%hhd' expects argument of type 'signed char *', but argument 3 has type 'int_fast8_t *' [-Werror=format=] Kannst Du Dir das bitte einmal ansehen? -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany) On 25 Jul 2014, at 23:43, Joerg Sonnenberger jo...@netbsd.org wrote: Module Name: src Committed By: joerg Date: Fri Jul 25 21:43:13 UTC 2014 Modified Files: src/distrib/sets/lists/comp: mi src/sys/arch/amd64/include: int_const.h int_fmtio.h int_limits.h int_mwgwtypes.h int_types.h src/sys/arch/arm/include: int_const.h int_fmtio.h int_limits.h int_mwgwtypes.h int_types.h src/sys/arch/i386/include: int_const.h int_fmtio.h int_limits.h int_mwgwtypes.h int_types.h src/sys/sys: Makefile Added Files: src/sys/sys: common_int_const.h common_int_fmtio.h common_int_limits.h common_int_mwgwtypes.h common_int_types.h Log Message: Add generic versions of machine/int_*.h for compilers providing appropiate macros for all necessary types. To generate a diff of this commit: cvs rdiff -u -r1.1903 -r1.1904 src/distrib/sets/lists/comp/mi cvs rdiff -u -r1.4 -r1.5 src/sys/arch/amd64/include/int_const.h cvs rdiff -u -r1.6 -r1.7 src/sys/arch/amd64/include/int_fmtio.h \ src/sys/arch/amd64/include/int_types.h cvs rdiff -u -r1.8 -r1.9 src/sys/arch/amd64/include/int_limits.h cvs rdiff -u -r1.7 -r1.8 src/sys/arch/amd64/include/int_mwgwtypes.h cvs rdiff -u -r1.3 -r1.4 src/sys/arch/arm/include/int_const.h cvs rdiff -u -r1.7 -r1.8 src/sys/arch/arm/include/int_fmtio.h cvs rdiff -u -r1.10 -r1.11 src/sys/arch/arm/include/int_limits.h cvs rdiff -u -r1.6 -r1.7 src/sys/arch/arm/include/int_mwgwtypes.h cvs rdiff -u -r1.16 -r1.17 src/sys/arch/arm/include/int_types.h cvs rdiff -u -r1.3 -r1.4 src/sys/arch/i386/include/int_const.h cvs rdiff -u -r1.7 -r1.8 src/sys/arch/i386/include/int_fmtio.h cvs rdiff -u -r1.8 -r1.9 src/sys/arch/i386/include/int_limits.h cvs rdiff -u -r1.6 -r1.7 src/sys/arch/i386/include/int_mwgwtypes.h cvs rdiff -u -r1.10 -r1.11 src/sys/arch/i386/include/int_types.h cvs rdiff -u -r1.148 -r1.149 src/sys/sys/Makefile cvs rdiff -u -r0 -r1.1 src/sys/sys/common_int_const.h \ src/sys/sys/common_int_fmtio.h src/sys/sys/common_int_limits.h \ src/sys/sys/common_int_mwgwtypes.h src/sys/sys/common_int_types.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Re: CVS commit: src
oops, this was meant for Joerg only ... Since this commit i386 doesn't build, pleas take a look. On 28 Jul 2014, at 09:40, J. Hannken-Illjes hann...@eis.cs.tu-bs.de wrote: Jörg, seit diesem Commit gibt es keine i386 Builds mehr: src/tests/include/t_inttypes.c:173:2: error: format '%hhd' expects argument of type 'signed char *', but argument 3 has type 'int_fast8_t *' [-Werror=format=] SCAN(SCNdFAST8, if8); ^ src/tests/include/t_inttypes.c:173:2: error: format '%hhd' expects argument of type 'signed char *', but argument 3 has type 'int_fast8_t *' [-Werror=format=] Kannst Du Dir das bitte einmal ansehen? -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany) On 25 Jul 2014, at 23:43, Joerg Sonnenberger jo...@netbsd.org wrote: Module Name: src Committed By:joerg Date:Fri Jul 25 21:43:13 UTC 2014 Modified Files: src/distrib/sets/lists/comp: mi src/sys/arch/amd64/include: int_const.h int_fmtio.h int_limits.h int_mwgwtypes.h int_types.h src/sys/arch/arm/include: int_const.h int_fmtio.h int_limits.h int_mwgwtypes.h int_types.h src/sys/arch/i386/include: int_const.h int_fmtio.h int_limits.h int_mwgwtypes.h int_types.h src/sys/sys: Makefile Added Files: src/sys/sys: common_int_const.h common_int_fmtio.h common_int_limits.h common_int_mwgwtypes.h common_int_types.h Log Message: Add generic versions of machine/int_*.h for compilers providing appropiate macros for all necessary types. To generate a diff of this commit: cvs rdiff -u -r1.1903 -r1.1904 src/distrib/sets/lists/comp/mi cvs rdiff -u -r1.4 -r1.5 src/sys/arch/amd64/include/int_const.h cvs rdiff -u -r1.6 -r1.7 src/sys/arch/amd64/include/int_fmtio.h \ src/sys/arch/amd64/include/int_types.h cvs rdiff -u -r1.8 -r1.9 src/sys/arch/amd64/include/int_limits.h cvs rdiff -u -r1.7 -r1.8 src/sys/arch/amd64/include/int_mwgwtypes.h cvs rdiff -u -r1.3 -r1.4 src/sys/arch/arm/include/int_const.h cvs rdiff -u -r1.7 -r1.8 src/sys/arch/arm/include/int_fmtio.h cvs rdiff -u -r1.10 -r1.11 src/sys/arch/arm/include/int_limits.h cvs rdiff -u -r1.6 -r1.7 src/sys/arch/arm/include/int_mwgwtypes.h cvs rdiff -u -r1.16 -r1.17 src/sys/arch/arm/include/int_types.h cvs rdiff -u -r1.3 -r1.4 src/sys/arch/i386/include/int_const.h cvs rdiff -u -r1.7 -r1.8 src/sys/arch/i386/include/int_fmtio.h cvs rdiff -u -r1.8 -r1.9 src/sys/arch/i386/include/int_limits.h cvs rdiff -u -r1.6 -r1.7 src/sys/arch/i386/include/int_mwgwtypes.h cvs rdiff -u -r1.10 -r1.11 src/sys/arch/i386/include/int_types.h cvs rdiff -u -r1.148 -r1.149 src/sys/sys/Makefile cvs rdiff -u -r0 -r1.1 src/sys/sys/common_int_const.h \ src/sys/sys/common_int_fmtio.h src/sys/sys/common_int_limits.h \ src/sys/sys/common_int_mwgwtypes.h src/sys/sys/common_int_types.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/fs/msdosfs
On 13 Jul 2014, at 15:36, Joerg Sonnenberger jo...@britannica.bec.de wrote: On Tue, Jul 08, 2014 at 09:21:52AM +, Juergen Hannken-Illjes wrote: Module Name: src Committed By:hannken Date:Tue Jul 8 09:21:52 UTC 2014 Modified Files: src/sys/fs/msdosfs: denode.h msdosfs_denode.c msdosfs_lookup.c msdosfs_vfsops.c msdosfs_vnops.c msdosfsmount.h Log Message: Change msdosfs from hashlist to vcache: - Use (dir_cluster, dir_offset, dir_generation) as key, where dir_generation is non-zero and unique for unlinked but open nodes. - Change deget() to return a vnode as it is unsafe to return a referenced but unlocked denode. This broke sysutils/lsof, can you please take a look? /usr/include/msdosfs/msdosfsmount.h:255:12: error: a parameter list without types is only allowed in a function definition VFS_PROTOS(msdosfs); Maybe a patch to lsof/dialects/n+obsd/dlsof.h like #if defined (NETBSDV) #define VFS_PROTOS(dummy) /**/ #endif before including msdosfsmount.h? -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/kern
On 14 Jun 2014, at 18:12, Joerg Sonnenberger jo...@netbsd.org wrote: Module Name: src Committed By: joerg Date: Sat Jun 14 16:12:34 UTC 2014 Modified Files: src/sys/kern: vfs_cache.c Log Message: Make the stat mutex a leaf. XXX Use atomic counters. This cannot work. We now run cache_lookup_entry() without a cpu_lock and without namecache_lock so the list may change while it gets traversed. See the comment on top of cache_lookup_entry(). Please revert your changes to vfs_cache.c. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys
On 03 Jun 2014, at 21:30, Joerg Sonnenberger jo...@netbsd.org wrote: Module Name: src Committed By: joerg Date: Tue Jun 3 19:30:30 UTC 2014 Modified Files: src/sys/fs/cd9660: cd9660_lookup.c src/sys/fs/filecorefs: filecore_lookup.c src/sys/kern: vfs_cache.c src/sys/sys: namei.src src/sys/ufs/ext2fs: ext2fs_lookup.c src/sys/ufs/lfs: ulfs_lookup.c src/sys/ufs/ufs: ufs_lookup.c Log Message: Introduce two helper functions to centralise the namecache statistics in vfs_cache.c. Use consistent locking around the per-cpu data. This change leads to a deadlock: Thread 1 calls cache_revlookup(), holds ncp-nc_lock @ line 589, wants cpup-cpu_lock @ line 604 Thread 2 calls cache_lookup(), holds cpup-cpu_lock @ line 391, calls cache_lookup_entry(), wants ncp-nc_lock @ line 308 -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/kern
On 15 Apr 2014, at 20:18, Taylor R Campbell campbell+netbsd-source-change...@mumble.net wrote: Date: Tue, 15 Apr 2014 09:50:45 + From: Juergen Hannken-Illjes hann...@netbsd.org Fix a deadlock where one thread exits, enters fstrans_lwp_dtor() and wants fstrans_lock. This thread holds the proc_lock. This sounds fishy. lwp_exit does not hold proc_lock while calling lwp_finispecific, so there are no invariants covered by proc_lock that the lwp_specific destructors can rely on. I'm inclined to say that it is a bug for exit1 to hold proc_lock when it calls lwp_finispecific (and proc_finispecific). Can we just release it before and re-acquire it after calling lwp/proc_finispecific? Another thread holds fstrans_lock and runs pserialize_perform(). As the first thread holds the proc_lock, timeouts are blocked and the second thread blocks forever in kpause(). This also sounds fishy. How does T1's holding proc_lock cause T2 to block forever in kpause? I think I'm missing something in this analysis. kpause doesn't take proc_lock, does it? Kpause() registers a callout and sleeps. On the next clock tick the softclk thread calls timer_intr() which wants the proc_lock. Now we successfully stopped all softclk threads. The callout for kpause() will not fire - deadlock/ -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src
On Feb 27, 2014, at 6:06 PM, Mindaugas Rasiukevicius rm...@netbsd.org wrote: Juergen Hannken-Illjes hann...@netbsd.org 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
On Feb 27, 2014, at 7:01 PM, Mindaugas Rasiukevicius rm...@netbsd.org wrote: J. Hannken-Illjes hann...@eis.cs.tu-bs.de 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
On Feb 27, 2014, at 4:56 PM, Joerg Sonnenberger jo...@netbsd.org 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
On Feb 6, 2014, at 4:45 PM, Taylor R Campbell
campbell+netbsd-source-change...@mumble.net wrote:
Date: Thu, 6 Feb 2014 10:57:13 +
From: Juergen Hannken-Illjes hann...@netbsd.org
Move fstrans_start()/fstrans_done() into genfs_insane_rename() to protect
the complete rename operation like we do for all other vnode operations.
I've always been a little fuzzy on the rules for using fstrans. When
I wrote genfs_rename, I was under the (probably incorrect) impression
that it was there to block writers gracefully so that the file system
could be put into a consistent state for a snapshot.
Could you explain what the rules for using fstrans_{start,done} are,
and how they relate to vnode locks, lock order, and I/O operations?
The fstrans(9) man page is a little terse.
The fstrans interface is just a helper for vfs_suspend(9):
vfs_suspend(mp, nowait)
Request a mounted file system to suspend all operations. All
new operations to the file system are stopped. After all
operations in progress have completed, the file system is synced
to disk and the function returns.
That means most vnode operations look like:
fs_op(void *v)
{
fstrans_start(mp, FSTRANS_SHARED);
...
fstrans_done(mp);
return error;
}
Exceptions are VOP_GETPAGES()/VOP_PUTPAGES() as they run with
v_interlock held and therefore cant sleep and operations going down
to specfs for example. Here we have the problem that the specfs
operation may take forever -- reading from a tty for example.
--
J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/share/mk
On Jan 21, 2014, at 5:40 PM, Matt Thomas m...@netbsd.org wrote: Module Name: src Committed By: matt Date: Tue Jan 21 16:40:24 UTC 2014 Modified Files: src/share/mk: bsd.own.mk Log Message: Make MKGCCCMDS default mirror MKGCC. (if MKGCC is no, MKGCCCMDS must be no). This breaks all builds as MKGCCCMDS is now missing from the set list print controls. See distrib/sets/mkvars.mk, target mkvars -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/fs/tmpfs
On Jan 8, 2014, at 5:11 PM, pedro martelletto pe...@netbsd.org wrote: Module Name: src Committed By: pedro Date: Wed Jan 8 16:11:04 UTC 2014 Modified Files: src/sys/fs/tmpfs: tmpfs_subr.c Log Message: Allocate direntp on the stack in tmpfs_dir_getdents(), thus saving calls to kmem_zalloc() and kmem_free(); OK rmind@. From OpenBSD. Is it really a good idea to allocate 528 bytes on the kernel stack? File systems nest and already use much stack space. Looks better to use a pool_cache. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/fs/tmpfs
On Jan 3, 2014, at 10:18 PM, J. Hannken-Illjes hann...@eis.cs.tu-bs.de wrote: On Jan 3, 2014, at 10:13 PM, Mindaugas Rasiukevicius rm...@netbsd.org wrote: Juergen Hannken-Illjes hann...@netbsd.org wrote: Module Name:src Committed By: hannken Date: Fri Jan 3 09:53:12 UTC 2014 Modified Files: src/sys/fs/tmpfs: tmpfs_subr.c tmpfs_vnops.c Log Message: Fix a race where thread1 runs VOP_REMOVE() and gets preempted in tmpfs_reclaim() before the call to tmpfs_free_node(). Thread2 runs VFS_FHTOVP() and gets a new vnode attached to the node thread1 is about to destroy. Change tmpfs_alloc_node() to always assign non-zero generation number and tmpfs_inactive() to set the generation number of unlinked nodes to zero. Can you explain how does this help? It still seems racy to me. Please describe the race in more detail. Tmpfs_fhtovp() will fail as soon as an unlinked tmpfs node drops its last vnode reference. Ok -- got it. We check the generation number too early in tmpfs_fhtovp(). Should be fixed with tmpfs_vfsops.c Rev. 1.55 -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/fs/tmpfs
On Jan 3, 2014, at 10:13 PM, Mindaugas Rasiukevicius rm...@netbsd.org wrote: Juergen Hannken-Illjes hann...@netbsd.org wrote: Module Name: src Committed By:hannken Date:Fri Jan 3 09:53:12 UTC 2014 Modified Files: src/sys/fs/tmpfs: tmpfs_subr.c tmpfs_vnops.c Log Message: Fix a race where thread1 runs VOP_REMOVE() and gets preempted in tmpfs_reclaim() before the call to tmpfs_free_node(). Thread2 runs VFS_FHTOVP() and gets a new vnode attached to the node thread1 is about to destroy. Change tmpfs_alloc_node() to always assign non-zero generation number and tmpfs_inactive() to set the generation number of unlinked nodes to zero. Can you explain how does this help? It still seems racy to me. Please describe the race in more detail. Tmpfs_fhtovp() will fail as soon as an unlinked tmpfs node drops its last vnode reference. Why not just check for tn_links == 0 in tmpfs_fhtovp()? Because it is ok as long as the corresponding vnode is open/referenced. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/external/bsd/ntp/dist
On Dec 28, 2013, at 4:20 AM, Christos Zoulas chris...@netbsd.org wrote: Module Name: src Committed By: christos Date: Sat Dec 28 03:20:15 UTC 2013 snip src/external/bsd/ntp/dist/ntpd/ntp_parser.y \ snip Looks like this commit removed the keyword mdnstries so the provided /etc/ntp.conf fails as: ntpd[317]: line 33 column 1 syntax error, unexpected T_String, expecting $end ntpd[317]: syntax error in /etc/ntp.conf line 33, column 1 -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys
On Nov 28, 2013, at 2:53 PM, Mindaugas Rasiukevicius rm...@netbsd.org wrote: Hello Juergen, Juergen Hannken-Illjes hann...@netbsd.org wrote: Module Name: src Committed By:hannken Date:Sat Nov 23 13:46:22 UTC 2013 Modified Files: src/sys/kern: vfs_vnode.c src/sys/sys: vnode.h Log Message: Replace VI_INACTNOW and VI_INACTREDO with a new flag VI_CHANGING that gets set while a vnode changes state from active to inactive or from active or inactive to clean and protects vclean(); vrelel() and vrelel() against vget(). Presented on tech-kern. There are multiple reports of hangs in VFS (see something's wrong thread on current-users@ and PR/48411). It seems to be due to this change. Can you revert it for now? No -- please give me some hours more to investigate. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/fs/tmpfs
On Nov 8, 2013, at 4:44 PM, Mindaugas Rasiukevicius rm...@netbsd.org wrote: Module Name: src Committed By: rmind Date: Fri Nov 8 15:44:23 UTC 2013 Modified Files: src/sys/fs/tmpfs: tmpfs.h tmpfs_rename.c tmpfs_subr.c tmpfs_vfsops.c tmpfs_vnops.c Log Message: tmpfs: replace the broken tmpfs_dircookie() logic which uses the node address truncated to 31 bits (required for 32-bit readdir compatibility, e.g. linux32). Instead, assign 2^31 range using the following logic: - The first half of the 2^31 is assigned incrementally (the fast path). - When exceeded, use the second half of 2^31, but manage with vmem(9). It will require 2 billion files per-directory to trigger vmem(9) usage. Also, while here, add some fixes for tmpfs_unmount(). Should fix PR/47739, PR/47480, PR/46088 and PR/41068. Thanks to wiz@ for stress testing. The tests fs/vfs/t_union/tmpfs_basic and fs/vfs/t_union/tmpfs_whiteout start failing after this commit. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys
On Apr 27, 2013, at 12:27 AM, Michael van Elst mlel...@netbsd.org wrote: Module Name: src Committed By: mlelstv Date: Fri Apr 26 22:27:17 UTC 2013 Modified Files: src/sys/kern: vfs_mount.c src/sys/sys: mount.h Log Message: Correct umount semantics to return EBUSY when a filesystem is busy instead of failing filesystem operations with EBUSY when attempting an umount. This fixes kern/38141. Who did the review or where has it been discussed? We now get a deadlock between mountlist_lock and mnt_unmounting, seen between dounmount() and do_sys_sync() for example. The lock order is mountlist_lock-mnt_unmounting, but dounmount() locks in reverse direction. Please fix or revert. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys
On Apr 28, 2013, at 10:56 AM, Michael van Elst mlel...@serpens.de wrote: On Sun, Apr 28, 2013 at 10:31:19AM +0200, J. Hannken-Illjes wrote: On Apr 27, 2013, at 12:27 AM, Michael van Elst mlel...@netbsd.org wrote: Module Name:src Committed By: mlelstv Date: Fri Apr 26 22:27:17 UTC 2013 Modified Files: src/sys/kern: vfs_mount.c src/sys/sys: mount.h Log Message: Correct umount semantics to return EBUSY when a filesystem is busy instead of failing filesystem operations with EBUSY when attempting an umount. This fixes kern/38141. Who did the review or where has it been discussed? Christos with a former version and David Holland with the current. We now get a deadlock between mountlist_lock and mnt_unmounting, seen between dounmount() and do_sys_sync() for example. Do you have a PR showing how to produce that deadlock? No PR -- but the attached script should do the job. The lock order is mountlist_lock-mnt_unmounting, but dounmount() locks in reverse direction. The locking order hasn't been changed, but the faulty tryenter has been replaced with an enter. Sure -- and that is the problem. The locking order was wrong and you removed the hack/workaround without fixing the lock order. If there is a locking violation it just didn't show up with the former code as even LOCKDEBUG had been hacked to allow it. This hack was about recursive rw_tryenter() if I remember right. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany) hang-it.sh Description: Binary data
Re: CVS commit: src/sys
On Apr 28, 2013, at 4:32 PM, Michael van Elst mlel...@serpens.de wrote: On Sun, Apr 28, 2013 at 01:50:37PM +0200, J. Hannken-Illjes wrote: The locking order hasn't been changed, but the faulty tryenter has been replaced with an enter. Sure -- and that is the problem. The locking order was wrong and you removed the hack/workaround without fixing the lock order. I now release the mnt_unmounting lock before aquiring the mountlist_lock. This opens a race condition for a concurrent umount that I prevent by raising the nest counter. With this change I can no longer provoke the deadlock. The only difference should now be that vfs_hooks_unmount() is called outside the unmounting lock. But since it is operating on an already detached mountpoint (if it is used at all) before it is destroyed, that should be harmless. What do you think? snip Unfortunately this also opens a race for do_sys_sync to succeed with vfs_busy() and call ffs_sync() with mp-mnt_data == NULL - BOMB. -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/kern
On Dec 29, 2012, at 10:56 PM, Christos Zoulas chris...@netbsd.org wrote: Always call brelse() on error. Otherwise a possible error from bread() will cause the buffer to stay lock and we end up blocking forever in VOP_CLOSE-spec_close-vinvalbuf-bbysy since the buffer is marked busy but there is no I/O pending. This caused my laptop to hang on boot_findwedge because: findroot: unable to read block 358331527 of dev dk0 (22) Thanks. Just applied the same to breadn(). -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys
On Dec 27, 2012, at 11:46 PM, Taylor R Campbell riastr...@netbsd.org wrote:
From: hann...@netbsd.org
Date: Thu, 20 Dec 2012 08:03:46 +
--- sys/fs/msdosfs/msdosfs_vnops.c 29 Apr 2012 22:53:59 - 1.83
+++ sys/fs/msdosfs/msdosfs_vnops.c 20 Dec 2012 08:03:42 - 1.84
@@ -527,7 +527,6 @@ msdosfs_read(void *v)
NOCRED, 0, bp);
n = MIN(n, pmp-pm_bpcluster - bp-b_resid);
if (error) {
- brelse(bp, 0);
Looks like you need to move the initialization of n to after the error
branch.
Fixed, thanks.
--- sys/ufs/ext2fs/ext2fs_vfsops.c 21 Nov 2012 23:11:23 -
1.167
+++ sys/ufs/ext2fs/ext2fs_vfsops.c 20 Dec 2012 08:03:44 -
1.168
@@ -772,8 +770,8 @@ ext2fs_mountfs(struct vnode *devvp, stru
return (0);
out:
- KASSERT(bp != NULL);
- brelse(bp, 0);
+ if (bp != NULL)
+ brelse(bp, 0);
Are bread c. supposed to guarantee that bp is null on error, or are
they supposed to leave it in an undefined state? Either way, it seems
to me that the intent ought to be documented in buffercache(9).
Both bread() and breadn() will either return with `error == 0, *bpp != NULL'
or `error != 0, *bpp == NULL'.
--
J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/arch/x68k/conf
On Nov 3, 2012, at 3:44 PM, Izumi Tsutsui wrote: Module Name: src Committed By: tsutsui Date: Sat Nov 3 14:44:29 UTC 2012 Modified Files: src/sys/arch/x68k/conf: GENERIC Log Message: Comment out pseudo-device fss. We already has options FFS_NO_SNAPSHOT. This is not the same: - having fss makes it possible to use file system external or already configured snapshots while - FFS_NO_SNAPSHOT doesn't allow the creation of file system internal snapshots. It was requested to reduce the size of INSTALL kernels only. -- Juergen Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/arch/x68k/conf
On Nov 3, 2012, at 4:51 PM, Izumi Tsutsui wrote: hannken@ wrote: Modified Files: src/sys/arch/x68k/conf: GENERIC Log Message: Comment out pseudo-device fss. We already has options FFS_NO_SNAPSHOT. This is not the same: - having fss makes it possible to use file system external or already configured snapshots while - FFS_NO_SNAPSHOT doesn't allow the creation of file system internal snapshots. It was requested to reduce the size of INSTALL kernels only. Hmm. Then, is pseudo-device fss still useful with options FFS_NO_SNAPSHOT? (most x68k has only 8~12MB) Hmm -- depends, what is the size of fss.o here? -- Juergen Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/fs/union
On Nov 23, 2011, at 11:11 AM, Alan Barrett wrote: My vote would be to remove [unionfs]; it doesn't work and the only reason it was ever brought in had to do with alleged locking improvements. Is anyone using it? I used to make heavy use of unionfs, and I had no problems. (That was on a uniprocessor machine several years ago.) I sometimes used five layers: a base set of sources; a unionfs layer for third party changes; a unionfs layer for my own changes; a unionfs layer for the obj directories; and a final unionfs layer for files created or changed at build time. For example, I could easily blow away all the build products but keep the obj directories, by unmounting the top layer unionfs, removing the files in its backing store, and then re-mounting it. Today, I'd use a smarter revision control system instead of the unionfs layers to manage the source files, but I might still want a unionfs layer to isolate changes made at build time. I have not used unionfs in the past few years, but it would be a pity to lose this functionality. Do you mean `union'? `unionfs' was imported 2008/02/18 and was never enabled in any kernel config. -- Juergen Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/kern
On Nov 19, 2011, at 5:11 PM, Christos Zoulas wrote:
Module Name: src
Committed By: christos
Date: Sat Nov 19 16:11:24 UTC 2011
Modified Files:
src/sys/kern: cnmagic.c
Log Message:
PR/45633: Christian Biere: Don't access byte after NUL when setting magic.
This cannot work:
if (c == '\0')
return EINVAL;
switch (c) {
case '\0':
...
return 0; == This statement looks unreachable.
All attempts to set cnmagic result in EINVAL.
--
Juergen Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/kern
On Nov 20, 2011, at 6:10 PM, Christos Zoulas wrote:
On Nov 20, 10:02am, hann...@eis.cs.tu-bs.de (J. Hannken-Illjes) wrote:
-- Subject: Re: CVS commit: src/sys/kern
|
| On Nov 19, 2011, at 5:11 PM, Christos Zoulas wrote:
|
| Module Name: src
| Committed By: christos
| Date: Sat Nov 19 16:11:24 UTC 2011
|
| Modified Files:
|src/sys/kern: cnmagic.c
|
| Log Message:
| PR/45633: Christian Biere: Don't access byte after NUL when setting magic.
|
| This cannot work:
|
| if (c == '\0')
| return EINVAL;
| switch (c) {
| case '\0':
| ...
| return 0; == This statement looks unreachable.
|
| All attempts to set cnmagic result in EINVAL.
The patch was amended. Do you still have an issue?
No, Rev. 1.13 works fine -- sorry for the noise ...
--
Juergen Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/arch/i386/i386
On Oct 31, 2011, at 1:42 PM, YAMAMOTO Takashi wrote: Module Name: src Committed By: yamt Date: Mon Oct 31 12:42:53 UTC 2011 Modified Files: src/sys/arch/i386/i386: dumpsys.c Log Message: dumpsys_seg: don't overwrite the previous mapping With this change in place core dumps from ddb (reboot 104) no longer work on MP machines. Before pmap_tlb_shootnow() always returned on the `tp-tp_count == 0' check. Now it goes into the `remote' case and hangs hard trying to reach other CPUs. -- Juergen Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/nfs
On Nov 15, 2011, at 4:08 AM, YAMAMOTO Takashi wrote: hi, Module Name: src Committed By:hannken Date:Sun Oct 30 12:00:28 UTC 2011 Modified Files: src/sys/nfs: nfs_serv.c Log Message: VOP_GETATTR() needs a shared lock at least. this seems trying to lock the directory while holding its child locked. ie. wrong locking order Seems I overlooked one case, the change on top of nfsrv_lookup(). All other changes should use the right locking order. -- Juergen Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys
On Oct 15, 2011, at 3:30 AM, matthew green wrote: Module Name: src Committed By:hannken Date:Fri Oct 14 09:23:31 UTC 2011 Modified Files: src/sys/compat/linux/common: linux_file.c linux_file64.c linux_ioctl.c linux_misc.c src/sys/compat/linux32/common: linux32_dirent.c src/sys/compat/ossaudio: ossaudio.c src/sys/compat/svr4: svr4_fcntl.c src/sys/compat/svr4_32: svr4_32_fcntl.c src/sys/dev: ccd.c cgd.c vnd.c src/sys/dev/dm: dm_target_linear.c dm_target_snapshot.c dm_target_stripe.c src/sys/dev/raidframe: rf_copyback.c rf_disks.c rf_reconstruct.c src/sys/kern: kern_verifiedexec.c vfs_mount.c vfs_syscalls.c vfs_vnops.c vnode_if.src src/sys/uvm: uvm_mmap.c Log Message: Change the vnode locking protocol of VOP_GETATTR() to request at least a shared lock. Make all calls outside of file systems respect it. The calls from file systems need review. No objections from tech-kern. does this require a kernel bump? if filesystem code needs to change, even if API signatures haven't, i would say so? thanks. No, we don't check vnode locks for VOP_GETATTR() anywhere as its lock requirements were unspecified -- all changes are like + vn_lock(vp, LK_SHARED | LK_RETRY); error = VOP_GETATTR(vp, va, cred); + VOP_UNLOCK(vp); -- Juergen Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/dev/pci
On Oct 8, 2011, at 12:42 AM, Roland Dowdeswell wrote: Module Name: src Committed By: elric Date: Fri Oct 7 22:42:19 UTC 2011 Modified Files: src/sys/dev/pci: if_iwn.c This breaks building kernels: sys/dev/pci/if_iwn.c: In function 'iwn5000_runtime_calib': sys/dev/pci/if_iwn.c:4108:25: error: 'IWN5000_CALIB_DC' undeclared (first use in this function) sys/dev/pci/if_iwn.c:4108:25: note: each undeclared identifier is reported only once for each function it appears in sys/dev/pci/if_iwn.c: In function 'iwn_config': sys/dev/pci/if_iwn.c:4130:18: error: request for member 'dv_xname' in something not a structure or union -- Juergen Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/kern
On Oct 2, 2011, at 3:26 PM, Adam Hamsik wrote: On Oct,Sunday 2 2011, at 3:00 PM, Juergen Hannken-Illjes wrote: Module Name: src Committed By:hannken Date:Sun Oct 2 13:00:07 UTC 2011 Modified Files: src/sys/kern: vfs_vnode.c Log Message: The path getnewvnode()-getcleanvnode()-vclean()-VOP_LOCK() will panic if the vnode we want to clean is a layered vnode and the caller already locked its lower vnode. Change getnewvnode() to always allocate a fresh vnode and add a helper thread (vdrain) to keep the number of allocated vnodes within desiredvnodes. Rename getcleanvnode() to cleanvnode() and let it take a vnode from the lists, clean and free it. Thanks for doing this. This should help zfs too I saw couple of panics with zfs which were caused by this. Have you done any benchmarks ? when I proposed solution like this main objection was then before vnodes were not allocated from storage pool every time(sometime we reused already allocated one). And this may affect performance. I didn't run any benchmark. As getnewvnode() gets called when a file system sets up a new (not yet cached) vnode I suppose there are other operations taking much more time like reading an inode from disk, setting up the inode, creating the genfs_node etc. We also save the time spent in cleaning the vnode which is now done async to the allocation. Reviewed by: David Holland dholl...@netbsd.org Should fix: PR #19110 (nullfs mounts over NFS cause lock manager problems) PR #34102 (ffs panic in NetBSD 3.0_STABLE) PR #45115 (lock error panic when build.sh*3 and daily script is running) PR #45355 (Reader/writer lock error: rw_vector_enter: locking against myself) snip Regards Adam. -- Juergen Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/uvm
On Sep 29, 2011, at 12:52 AM, Matt Thomas wrote:
Module Name: src
Committed By: matt
Date: Wed Sep 28 22:52:16 UTC 2011
Modified Files:
src/sys/uvm: uvm_page.c uvm_pager.c uvm_pager.h
Log Message:
Reallocate emergency pager va when ncolors is increased. (modication of
patch from mrg).
This panics LOCKDEBUG kernels as the call to uvm_pager_realloc_emerg()
in function uvm_page_recolor(), file uvm_page.c:1048 with spin
lock uvm_fpageqlock held leads to:
Mutex error: lockdebug_barrier: spin lock held
db{0} bt
mutex_enter() at netbsd:mutex_enter+0x4a3
uvm_km_check_empty() at netbsd:uvm_km_check_empty+0x73
uvm_map() at netbsd:uvm_map+0x1a2
uvm_km_alloc() at netbsd:uvm_km_alloc+0xe4
uvm_pager_realloc_emerg() at netbsd:uvm_pager_realloc_emerg+0x5b
uvm_page_recolor() at netbsd:uvm_page_recolor+0x35b
cpu_attach() at netbsd:cpu_attach+0x333
--
Juergen Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/kern
On Aug 29, 2011, at 2:39 AM, Mindaugas Rasiukevicius wrote: Module Name: src Committed By: rmind Date: Mon Aug 29 00:39:16 UTC 2011 Modified Files: src/sys/kern: sys_select.c Log Message: Add kern.direct_select sysctl. Default to 0 for now. Why is it disabled by default? -- Juergen Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/distrib/alpha/instkernel/ramdisk
Looks like an error in the `list' file: SPECIAL pingsrcdir distrib/utils/x_gzip SPECIAL pingsrcdir distrib/utils/x_ping -- Juergen Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany) On Jun 21, 2011, at 1:36 AM, Christos Zoulas wrote: In article 20110620230053.9893c17...@cvs.netbsd.org, Havard Eidnes source-changes-d@NetBSD.org wrote: -=-=-=-=-=- Module Name: src Committed By:he Date:Mon Jun 20 23:00:53 UTC 2011 Modified Files: src/distrib/alpha/instkernel/ramdisk: list Log Message: Add -lbz2 and -llzma so that this links again. But that should not be needed for x_gzip. What needs those? christos
