[PATCH] Fix 'implicit declaration' warning and update vgone(9)
The vgonel function isnt declarated in any header, the vgonel prototype in vgone(9) isnt correct - found by Ben Kaduk ka...@mit.edu -- Lars Hartmann l...@chaotika.org fixed vgonel prototype in vgone.9 - found by Ben Kaduk ka...@mit.edu From: Lars Hartmann l...@chaotika.org --- share/man/man9/vgone.9 |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/share/man/man9/vgone.9 b/share/man/man9/vgone.9 index fa30c1e..a23d468 100644 --- a/share/man/man9/vgone.9 +++ b/share/man/man9/vgone.9 @@ -38,7 +38,7 @@ .Ft void .Fn vgone struct vnode *vp .Ft void -.Fn vgonel struct vnode *vp struct thread *td +.Fn vgonel struct vnode *vp .Sh DESCRIPTION .Fn vgone and fix implicit declaration warning - found by Ben Kaduk ka...@mit.edu From: Lars Hartmann l...@chaotika.org --- sys/sys/vnode.h |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/sys/sys/vnode.h b/sys/sys/vnode.h index e82f8ea..8ec8d1f 100644 --- a/sys/sys/vnode.h +++ b/sys/sys/vnode.h @@ -626,6 +626,7 @@ void vdestroy(struct vnode *); int vflush(struct mount *mp, int rootrefs, int flags, struct thread *td); int vget(struct vnode *vp, int lockflag, struct thread *td); void vgone(struct vnode *vp); +void vgone(struct vnode *vp); void vhold(struct vnode *); void vholdl(struct vnode *); int vinvalbuf(struct vnode *vp, int save, int slpflag, int slptimeo); ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: [PATCH] Fix 'implicit declaration' warning and update vgone(9)
attached the fixed vgonel.patch -- Lars Hartmann l...@chaotika.org fix implicit declaration warning - found by Ben Kaduk ka...@mit.edu From: Lars Hartmann l...@chaotika.org --- sys/sys/vnode.h |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/sys/sys/vnode.h b/sys/sys/vnode.h index e82f8ea..8ec8d1f 100644 --- a/sys/sys/vnode.h +++ b/sys/sys/vnode.h @@ -626,6 +626,7 @@ void vdestroy(struct vnode *); int vflush(struct mount *mp, int rootrefs, int flags, struct thread *td); int vget(struct vnode *vp, int lockflag, struct thread *td); void vgone(struct vnode *vp); +void vgonel(struct vnode *vp); void vhold(struct vnode *); void vholdl(struct vnode *); int vinvalbuf(struct vnode *vp, int save, int slpflag, int slptimeo); ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: fsync(2) manual and hdd write caching
Ivan Voras ivo...@freebsd.org wrote: fsync(2) actually does behave as advertised, auses all modified data and attributes of fd to be moved to a permanent storage device. It is the problem of the permanent storage device if it caches this data further. IMO, volatile RAM without battery backup cannot reasonably be considered a permanent storage device, regardless of where it is physically located. Short of mounting synchronously, with the attendant performance hit, would it not make sense for fsync(2) to issue ATA_FLUSHCACHE or SCSI SYNCHRONIZE CACHE after it has finished writing data to the drive? Surely the low-level capability to issue those commands must already exist, else we would have no way to safely prepare for power off. ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: fsync(2) manual and hdd write caching
On Wed, 27 Oct 2010 02:00:51 -0700 per...@pluto.rain.com wrote: Short of mounting synchronously, with the attendant performance hit, would it not make sense for fsync(2) to issue ATA_FLUSHCACHE or SCSI SYNCHRONIZE CACHE after it has finished writing data to the drive? Surely the low-level capability to issue those commands must already exist, else we would have no way to safely prepare for power off. mounting synchronously won't help, will it? As I understand it that just makes sure that data is sent straight to disk and not left in memory; the data will still be stored in the HDD cache for a while. -- Bruce Cran ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: fsync(2) manual and hdd write caching
On 10/27/10 12:11, Bruce Cran wrote: On Wed, 27 Oct 2010 02:00:51 -0700 per...@pluto.rain.com wrote: Short of mounting synchronously, with the attendant performance hit, would it not make sense for fsync(2) to issue ATA_FLUSHCACHE or SCSI SYNCHRONIZE CACHE after it has finished writing data to the drive? Surely the low-level capability to issue those commands must already exist, else we would have no way to safely prepare for power off. mounting synchronously won't help, will it? As I understand it that just makes sure that data is sent straight to disk and not left in memory; the data will still be stored in the HDD cache for a while. Correct. The problem is actually pretty hard - since AFAIK SoftUpdates doesn't have checkpoints in the sense that it groups writes and all data before can guaranteed to be on-disk, the problem is *when* to issue BIO_FLUSH requests. One possible solution is to simply decide on a heuristic like: ok, doing BIO_FLUSH all the time will destroy performance, we will only do it for every metadata write. Possibly with a sysctl tunable or per-mount option. ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: SYSCALL_MODULE() macro and modfind() issues
On 26 October 2010 17:34, John Baldwin j...@freebsd.org wrote: On Tuesday, October 26, 2010 4:00:14 am Selphie Keller wrote: Thanks Andriy, Took a look at the change to src/sys/sys/sysent.h @@ -149,7 +149,7 @@ static struct syscall_module_data name## }; \ \ static moduledata_t name##_mod = { \ - #name, \ + sys/ #name, \ syscall_module_handler, \ name##_syscall_mod \ }; \ applied the MFC prefix to pmap port: --- /usr/ports/sysutils/pmap/work/pmap/pmap/pmap.c.orig 2010-10-26 00:55:32.0 -0700 +++ /usr/ports/sysutils/pmap/work/pmap/pmap/pmap.c 2010-10-26 00:56:10.0 -0700 @@ -86,12 +86,12 @@ main(int argc, char **argv) struct kinfo_proc *kp; int pmap_helper_syscall; - if ((modid = modfind(pmap_helper)) == -1) { + if ((modid = modfind(sys/pmap_helper)) == -1) { /* module not found, try to load */ modid = kldload(pmap_helper.ko); if (modid == -1) err(1, unable to load pmap_helper module); - modid = modfind(pmap_helper); + modid = modfind(sys/pmap_helper); if (modid == -1) err(1, pmap_helper module loaded but not found); } which restored functionality on freebsd 8.1. The best approach might be to have something like this: static int pmap_find(void) { int modid; modid = modfind(pmap_helper); if (modid == -1) modid = modfind(sys/pmap_helper); return (modid); } then in the original main() routine use this: if ((modid = pmap_find()) == -1) { /* module not found, try to load */ modid = kldload(pmap_helper.ko); if (modid == -1) err(1, unable to load pmap_helper module); modid = pmap_find(); if (modid == -1) err(1, pmap_helper module loaded but not found); } This would make the code work for both old and new versions. Just another foo of many which I use at work generally. It lacks compat32 syscalls handling though (we don't use them). /* * We have to extract __FreeBSD_version from live kernel * as we depend on kernel feature and can run on an older world. */ if (sysctlbyname(kern.osreldate, osreldate, intlen, NULL, 0) 0) err(-2, sysctl(kern.osreldate)); if (osreldate = 800505)/* See r206346 in stable/8. */ strcpy(modname, sys/foo); else strcpy(modname, foo); -- wbr, pluknet ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: [PATCH] Fix 'implicit declaration' warning and update vgone(9)
On 27 October 2010 10:23, Lars Hartmann l...@chaotika.org wrote: The vgonel function isnt declarated in any header, the vgonel prototype in vgone(9) isnt correct - found by Ben Kaduk ka...@mit.edu Hi. I'm afraid it's just an overlooked man page after many VFS changes in 5.x. As vgonel() is a static (i.e. private and not visible from outside) function IMO it should be removed from vgone(9) man page. -- wbr, pluknet ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: Summary: Re: Spin down HDD after disk sync or before power off
On Thu Oct 21 10, Alexander Best wrote: On Thu Oct 21 10, Bruce Cran wrote: On Thu, 21 Oct 2010 14:33:49 +0200 Dag-Erling Smørgrav d...@des.no wrote: The problem with setting a short idle timeout is that, on a typical laptop or desktop system, you end up spinning the disk down and back up several hundred times a day, which increases power consumption, I/O latency and wear. Do we think our users are silly enough to set a short timeout of just a few minutes? I'd think most would use a setting of 20-30 minutes at a minimum. I never did understand why there were so many warnings; after all, some laptops even come with a default APM scheme in their HDDs that powers the disk down after 7 seconds! personally i still think something like the attached patch would be nice to have. there's a chance users might type the following: 'atacontrol spindown device 10' thinking the timeout value is measured in minutes. although this gets mentioned in atacontrol(4) it might still be worth reminding the user that he/she is performing actions which could damage the hardware. i just stumbled upon PR 144770, where a somebody seems to have mistaken the spindown value for minutes instead of seconds. so i really think we should have this warning in atacontrol! +1 from brucec, if i understood him correctly. another possibility would be of course changing the spindown value from seconds to minutes. imo this seems very reasonable, because measuring spindown time in seconds is too fine grained and not intuitive. just like specifying the 'shutdown -p XX' delay in microseconds would not be useful. ;) cheers. alex cheers. alex -- Bruce Cran -- a13x diff --git a/sbin/atacontrol/atacontrol.c b/sbin/atacontrol/atacontrol.c index 4354ddf..75a131a 100644 --- a/sbin/atacontrol/atacontrol.c +++ b/sbin/atacontrol/atacontrol.c @@ -317,6 +317,10 @@ ata_spindown(int fd, const char *dev, const char *arg) if (arg != NULL) { tmo = strtoul(arg, NULL, 0); + if (tmo 600) + warnx(setting spindown timeout below 10 minutes is \ + not recommended (see EXAMPLES section of \ + atacontrol(8))\n); if (ioctl(fd, IOCATASSPINDOWN, tmo) 0) err(1, ioctl(IOCATASSPINDOWN)); } else { -- a13x ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: Summary: Re: Spin down HDD after disk sync or before power off
On Wed, Oct 27, 2010 at 5:41 AM, Alexander Best arun...@freebsd.org wrote: On Thu Oct 21 10, Alexander Best wrote: On Thu Oct 21 10, Bruce Cran wrote: On Thu, 21 Oct 2010 14:33:49 +0200 Dag-Erling Smørgrav d...@des.no wrote: The problem with setting a short idle timeout is that, on a typical laptop or desktop system, you end up spinning the disk down and back up several hundred times a day, which increases power consumption, I/O latency and wear. Do we think our users are silly enough to set a short timeout of just a few minutes? I'd think most would use a setting of 20-30 minutes at a minimum. I never did understand why there were so many warnings; after all, some laptops even come with a default APM scheme in their HDDs that powers the disk down after 7 seconds! personally i still think something like the attached patch would be nice to have. there's a chance users might type the following: 'atacontrol spindown device 10' thinking the timeout value is measured in minutes. although this gets mentioned in atacontrol(4) it might still be worth reminding the user that he/she is performing actions which could damage the hardware. i just stumbled upon PR 144770, where a somebody seems to have mistaken the spindown value for minutes instead of seconds. so i really think we should have this warning in atacontrol! +1 from brucec, if i understood him correctly. another possibility would be of course changing the spindown value from seconds to minutes. imo this seems very reasonable, because measuring spindown time in seconds is too fine grained and not intuitive. just like specifying the 'shutdown -p XX' delay in microseconds would not be useful. ;) cheers. alex cheers. alex -- Bruce Cran -- a13x diff --git a/sbin/atacontrol/atacontrol.c b/sbin/atacontrol/atacontrol.c index 4354ddf..75a131a 100644 --- a/sbin/atacontrol/atacontrol.c +++ b/sbin/atacontrol/atacontrol.c @@ -317,6 +317,10 @@ ata_spindown(int fd, const char *dev, const char *arg) if (arg != NULL) { tmo = strtoul(arg, NULL, 0); + if (tmo 600) + warnx(setting spindown timeout below 10 minutes is \ + not recommended (see EXAMPLES section of \ + atacontrol(8))\n); if (ioctl(fd, IOCATASSPINDOWN, tmo) 0) err(1, ioctl(IOCATASSPINDOWN)); } else { Why not just be consistent with other interfaces and provide suffixes for the values to parse out integral times (i.e. 1 [second], 1m, 2h)? As long as the value is behavior is properly documented in the manpage (and potentially as examples in the usage message), that should be enough. Thanks, -Garrett ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: Summary: Re: Spin down HDD after disk sync or before power off
On Wed Oct 27 10, Garrett Cooper wrote: On Wed, Oct 27, 2010 at 5:41 AM, Alexander Best arun...@freebsd.org wrote: On Thu Oct 21 10, Alexander Best wrote: On Thu Oct 21 10, Bruce Cran wrote: On Thu, 21 Oct 2010 14:33:49 +0200 Dag-Erling Smørgrav d...@des.no wrote: The problem with setting a short idle timeout is that, on a typical laptop or desktop system, you end up spinning the disk down and back up several hundred times a day, which increases power consumption, I/O latency and wear. Do we think our users are silly enough to set a short timeout of just a few minutes? I'd think most would use a setting of 20-30 minutes at a minimum. I never did understand why there were so many warnings; after all, some laptops even come with a default APM scheme in their HDDs that powers the disk down after 7 seconds! personally i still think something like the attached patch would be nice to have. there's a chance users might type the following: 'atacontrol spindown device 10' thinking the timeout value is measured in minutes. although this gets mentioned in atacontrol(4) it might still be worth reminding the user that he/she is performing actions which could damage the hardware. i just stumbled upon PR 144770, where a somebody seems to have mistaken the spindown value for minutes instead of seconds. so i really think we should have this warning in atacontrol! +1 from brucec, if i understood him correctly. another possibility would be of course changing the spindown value from seconds to minutes. imo this seems very reasonable, because measuring spindown time in seconds is too fine grained and not intuitive. just like specifying the 'shutdown -p XX' delay in microseconds would not be useful. ;) cheers. alex cheers. alex -- Bruce Cran -- a13x diff --git a/sbin/atacontrol/atacontrol.c b/sbin/atacontrol/atacontrol.c index 4354ddf..75a131a 100644 --- a/sbin/atacontrol/atacontrol.c +++ b/sbin/atacontrol/atacontrol.c @@ -317,6 +317,10 @@ ata_spindown(int fd, const char *dev, const char *arg) if (arg != NULL) { tmo = strtoul(arg, NULL, 0); + if (tmo 600) + warnx(setting spindown timeout below 10 minutes is \ + not recommended (see EXAMPLES section of \ + atacontrol(8))\n); if (ioctl(fd, IOCATASSPINDOWN, tmo) 0) err(1, ioctl(IOCATASSPINDOWN)); } else { Why not just be consistent with other interfaces and provide suffixes for the values to parse out integral times (i.e. 1 [second], 1m, 2h)? As long as the value is behavior is properly documented in the manpage (and potentially as examples in the usage message), that should be enough. that would increase usability, since users don't have to specify large values in seconds, if they e.g. want the spindown to happen after 24 hours (24*60*60). but this will not solve the real issue: specifying 'atacontrol spindown 1s' WILL damage your hardware! imo users should be reminded about this even if this is already mentioned in the manual. also: will current scripts which set 'atacontrol spindown 600' e.g. continue to work after implementing the use of suffixes? Thanks, -Garrett -- a13x ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: Summary: Re: Spin down HDD after disk sync or before power off
On Wed, Oct 27, 2010 at 5:54 AM, Alexander Best arun...@freebsd.org wrote: On Wed Oct 27 10, Garrett Cooper wrote: On Wed, Oct 27, 2010 at 5:41 AM, Alexander Best arun...@freebsd.org wrote: On Thu Oct 21 10, Alexander Best wrote: On Thu Oct 21 10, Bruce Cran wrote: On Thu, 21 Oct 2010 14:33:49 +0200 Dag-Erling Smørgrav d...@des.no wrote: The problem with setting a short idle timeout is that, on a typical laptop or desktop system, you end up spinning the disk down and back up several hundred times a day, which increases power consumption, I/O latency and wear. Do we think our users are silly enough to set a short timeout of just a few minutes? I'd think most would use a setting of 20-30 minutes at a minimum. I never did understand why there were so many warnings; after all, some laptops even come with a default APM scheme in their HDDs that powers the disk down after 7 seconds! personally i still think something like the attached patch would be nice to have. there's a chance users might type the following: 'atacontrol spindown device 10' thinking the timeout value is measured in minutes. although this gets mentioned in atacontrol(4) it might still be worth reminding the user that he/she is performing actions which could damage the hardware. i just stumbled upon PR 144770, where a somebody seems to have mistaken the spindown value for minutes instead of seconds. so i really think we should have this warning in atacontrol! +1 from brucec, if i understood him correctly. another possibility would be of course changing the spindown value from seconds to minutes. imo this seems very reasonable, because measuring spindown time in seconds is too fine grained and not intuitive. just like specifying the 'shutdown -p XX' delay in microseconds would not be useful. ;) cheers. alex cheers. alex -- Bruce Cran -- a13x diff --git a/sbin/atacontrol/atacontrol.c b/sbin/atacontrol/atacontrol.c index 4354ddf..75a131a 100644 --- a/sbin/atacontrol/atacontrol.c +++ b/sbin/atacontrol/atacontrol.c @@ -317,6 +317,10 @@ ata_spindown(int fd, const char *dev, const char *arg) if (arg != NULL) { tmo = strtoul(arg, NULL, 0); + if (tmo 600) + warnx(setting spindown timeout below 10 minutes is \ + not recommended (see EXAMPLES section of \ + atacontrol(8))\n); if (ioctl(fd, IOCATASSPINDOWN, tmo) 0) err(1, ioctl(IOCATASSPINDOWN)); } else { Why not just be consistent with other interfaces and provide suffixes for the values to parse out integral times (i.e. 1 [second], 1m, 2h)? As long as the value is behavior is properly documented in the manpage (and potentially as examples in the usage message), that should be enough. that would increase usability, since users don't have to specify large values in seconds, if they e.g. want the spindown to happen after 24 hours (24*60*60). but this will not solve the real issue: specifying 'atacontrol spindown 1s' WILL damage your hardware! imo users should be reminded about this even if this is already mentioned in the manual. Maybe something similar to kern.geom.debugflags should be implemented for this feature as a safety belt for people doing something stupid? also: will current scripts which set 'atacontrol spindown 600' e.g. continue to work after implementing the use of suffixes? Yeah... why not :)? Thanks! -Garrett ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: Summary: Re: Spin down HDD after disk sync or before power off
Alexander Best wrote: i just stumbled upon PR 144770, where a somebody seems to have mistaken the spindown value for minutes instead of seconds. so i really think we should have this warning in atacontrol! +1 from brucec, if i understood him correctly. another possibility would be of course changing the spindown value from seconds to minutes. imo this seems very reasonable, because measuring spindown time in seconds is too fine grained and not intuitive. just like specifying the 'shutdown -p XX' delay in microseconds would not be useful. ;) No, please don't. Changing the meaning of the value from seconds to minutes will break people's existing setups. I'm also against printing a warning for values less than 600. If I want to set the value to 300, I don't want to be slapped with a useless warning. Also note that 300 (and even much less) might be perfectly reasonable, depending on the kind of device! As a rule of thumb, the smaller a drive, the better it is optimized for many spin up/down cycles. For example, I've got a portable HDD mp3 player that contains a 1.8 disk. When playing an mp3 file, it spins up every 20 seconds or so, reads 500 KB, then spins down again. In my opinion, this is purely a documentation issue. If it is well-documented that the spindown value is given in seconds, then that should be sufficient. Supporting values like 5m or 1h would be fine, as long as a value without a letter still means seconds. Best regards Oliver -- Oliver Fromme, secnetix GmbH Co. KG, Marktplatz 29, 85567 Grafing b. M. Handelsregister: Registergericht Muenchen, HRA 74606, Geschäftsfuehrung: secnetix Verwaltungsgesellsch. mbH, Handelsregister: Registergericht Mün- chen, HRB 125758, Geschäftsführer: Maik Bachmann, Olaf Erb, Ralf Gebhart FreeBSD-Dienstleistungen, -Produkte und mehr: http://www.secnetix.de/bsd Python is an experiment in how much freedom programmers need. Too much freedom and nobody can read another's code; too little and expressiveness is endangered. -- Guido van Rossum ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: Summary: Re: Spin down HDD after disk sync or before power off
Alexander Best wrote: but this will not solve the real issue: specifying 'atacontrol spindown 1s' WILL damage your hardware! You're making assumptions. I can very well imagine scenarios where 1s might make sense and will not damage the hardware, for example when there is no file system mounted from the disk in question. For several years I ran a BBS machine that ran off a memory disk. Every 24 hours it made a backup of its current state by writing a tar file to the raw device of a physical disk (there were no file systems mounted from that disk). Having a spindown time of 1s in such a context certainly wouldn't hurt. Best regards Oliver PS: Please allow me to copy a few things from my collection of quotes which fit very well here: UNIX was not designed to stop you from doing stupid things, because that would also stop you from doing clever things. -- Doug Gwyn If you aim the gun at your foot and pull the trigger, it's UNIX's job to ensure reliable delivery of the bullet to where you aimed the gun (in this case, Mr. Foot). -- Terry Lambert Unix gives you just enough rope to hang yourself -- and then a couple of more feet, just to be sure. -- Eric Allman -- Oliver Fromme, secnetix GmbH Co. KG, Marktplatz 29, 85567 Grafing b. M. Handelsregister: Registergericht Muenchen, HRA 74606, Geschäftsfuehrung: secnetix Verwaltungsgesellsch. mbH, Handelsregister: Registergericht Mün- chen, HRB 125758, Geschäftsführer: Maik Bachmann, Olaf Erb, Ralf Gebhart FreeBSD-Dienstleistungen, -Produkte und mehr: http://www.secnetix.de/bsd I suggested holding a Python Object Oriented Programming Seminar, but the acronym was unpopular. -- Joseph Strout ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: [PATCH] Fix 'implicit declaration' warning and update vgone(9)
On 27 October 2010 15:33, Sergey Kandaurov pluk...@gmail.com wrote: On 27 October 2010 10:23, Lars Hartmann l...@chaotika.org wrote: The vgonel function isnt declarated in any header, the vgonel prototype in vgone(9) isnt correct - found by Ben Kaduk ka...@mit.edu Hi. I'm afraid it's just an overlooked man page after many VFS changes in 5.x. As vgonel() is a static (i.e. private and not visible from outside) function IMO it should be removed from vgone(9) man page. Something like this I'd like to check in. Comments? Index: ObsoleteFiles.inc === --- ObsoleteFiles.inc (revision 214414) +++ ObsoleteFiles.inc (working copy) @@ -14,6 +14,8 @@ # The file is partitioned: OLD_FILES first, then OLD_LIBS and OLD_DIRS last. # +# 20101027: vgonel(9) has gone to private API a while ago +OLD_FILES+=usr/share/man/man9/vgonel.9.gz # 20101020: catch up with vm_page_sleep_if_busy rename OLD_FILES+=usr/share/man/man9/vm_page_sleep_busy.9.gz # 20101011: removed subblock.h from liblzma Index: share/man/man9/Makefile === --- share/man/man9/Makefile (revision 214413) +++ share/man/man9/Makefile (working copy) @@ -1317,7 +1317,6 @@ vfs_getopt.9 vfs_setopt_part.9 \ vfs_getopt.9 vfs_setopts.9 MLINKS+=VFS_LOCK_GIANT.9 VFS_UNLOCK_GIANT.9 -MLINKS+=vgone.9 vgonel.9 MLINKS+=vhold.9 vdrop.9 \ vhold.9 vdropl.9 \ vhold.9 vholdl.9 Index: share/man/man9/vgone.9 === --- share/man/man9/vgone.9 (revision 214413) +++ share/man/man9/vgone.9 (working copy) @@ -26,24 +26,21 @@ .\ .\ $FreeBSD$ .\ -.Dd November 21, 2001 +.Dd October 27, 2010 .Dt VGONE 9 .Os .Sh NAME -.Nm vgone , vgonel +.Nm vgone .Nd prepare a vnode for reuse .Sh SYNOPSIS .In sys/param.h .In sys/vnode.h .Ft void .Fn vgone struct vnode *vp -.Ft void -.Fn vgonel struct vnode *vp struct thread *td .Sh DESCRIPTION +The .Fn vgone -and -.Fn vgonel -prepare a vnode for reuse by another file system. +function prepares a vnode for reuse by another file system. The preparation includes the cleaning of all file system specific data and the removal from its mount point vnode list. .Pp @@ -55,17 +52,11 @@ as in most cases the vnode is about to be reused, or its file system is being unmounted. .Pp -The difference between +The .Fn vgone -and -.Fn vgonel -is that -.Fn vgone -locks the vnode interlock and then calls -.Fn vgonel -while -.Fn vgonel -expects the interlock to already be locked. +function takes an unlocked vnode and returns with the vnode unlocked. +.Sh SEE ALSO +.Xr vnode 9 .Sh AUTHORS This manual page was written by .An Chad David Aq dav...@acns.ab.ca . Index: share/man/man9/vflush.9 === --- share/man/man9/vflush.9 (revision 214413) +++ share/man/man9/vflush.9 (working copy) @@ -75,7 +75,6 @@ will be returned. .Sh SEE ALSO .Xr vgone 9 , -.Xr vgonel 9 , .Xr vrele 9 .Sh AUTHORS This manual page was written by -- wbr, pluknet ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: [PATCH] Fix 'implicit declaration' warning and update vgone(9)
On Wednesday, October 27, 2010 7:33:13 am Sergey Kandaurov wrote: On 27 October 2010 10:23, Lars Hartmann l...@chaotika.org wrote: The vgonel function isnt declarated in any header, the vgonel prototype in vgone(9) isnt correct - found by Ben Kaduk ka...@mit.edu Hi. I'm afraid it's just an overlooked man page after many VFS changes in 5.x. As vgonel() is a static (i.e. private and not visible from outside) function IMO it should be removed from vgone(9) man page. Agreed. It certainly should not be added to vnode.h. I'm curious how the reporter is getting a warning since there is a static prototype for vgonel() in vfs_subr.c. -- John Baldwin ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: Summary: Re: Spin down HDD after disk sync or before power off
Alexander Best wrote: On Wed Oct 27 10, Garrett Cooper wrote: Why not just be consistent with other interfaces and provide suffixes for the values to parse out integral times (i.e. 1 [second], 1m, 2h)? As long as the value is behavior is properly documented in the manpage (and potentially as examples in the usage message), that should be enough. that would increase usability, since users don't have to specify large values in seconds, if they e.g. want the spindown to happen after 24 hours (24*60*60). Largest value that could be set more or less precisely for ATA disks is 5.5hours. Absolute maximum is 12 hours. So 24h interval is just inapplicable. but this will not solve the real issue: specifying 'atacontrol spindown 1s' WILL damage your hardware! imo users should be reminded about this even if this is already mentioned in the manual. I would say WILL depends on hardware. And it's reasonability also depends on situation. -- Alexander Motin ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: Summary: Re: Spin down HDD after disk sync or before power off
On Wed, 27 Oct 2010 15:04:39 +0200 (CEST) Oliver Fromme o...@lurza.secnetix.de wrote: I'm also against printing a warning for values less than 600. If I want to set the value to 300, I don't want to be slapped with a useless warning. Having just checked Windows and seen that it lets you specify a timeout down to 1 minute with no warnings, I don't think we want to make it more difficult for people to do the same thing on FreeBSD. I don't know if atacontrol already does this, but maybe we could have a log entry, for example: atacontrol /dev/ad0 spindown 60 spin-down timer set to 60 seconds. -- Bruce Cran ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: [PATCH] Fix 'implicit declaration' warning and update vgone(9)
On Wed, Oct 27, 2010 at 05:26:13PM +0400, Sergey Kandaurov wrote: On 27 October 2010 15:33, Sergey Kandaurov pluk...@gmail.com wrote: On 27 October 2010 10:23, Lars Hartmann l...@chaotika.org wrote: The vgonel function isnt declarated in any header, the vgonel prototype in vgone(9) isnt correct - found by Ben Kaduk ka...@mit.edu Hi. I'm afraid it's just an overlooked man page after many VFS changes in 5.x. As vgonel() is a static (i.e. private and not visible from outside) function IMO it should be removed from vgone(9) man page. Something like this I'd like to check in. Comments? Index: ObsoleteFiles.inc === --- ObsoleteFiles.inc (revision 214414) +++ ObsoleteFiles.inc (working copy) @@ -14,6 +14,8 @@ # The file is partitioned: OLD_FILES first, then OLD_LIBS and OLD_DIRS last. # +# 20101027: vgonel(9) has gone to private API a while ago +OLD_FILES+=usr/share/man/man9/vgonel.9.gz # 20101020: catch up with vm_page_sleep_if_busy rename OLD_FILES+=usr/share/man/man9/vm_page_sleep_busy.9.gz # 20101011: removed subblock.h from liblzma Index: share/man/man9/Makefile === --- share/man/man9/Makefile (revision 214413) +++ share/man/man9/Makefile (working copy) @@ -1317,7 +1317,6 @@ vfs_getopt.9 vfs_setopt_part.9 \ vfs_getopt.9 vfs_setopts.9 MLINKS+=VFS_LOCK_GIANT.9 VFS_UNLOCK_GIANT.9 -MLINKS+=vgone.9 vgonel.9 MLINKS+=vhold.9 vdrop.9 \ vhold.9 vdropl.9 \ vhold.9 vholdl.9 Index: share/man/man9/vgone.9 === --- share/man/man9/vgone.9 (revision 214413) +++ share/man/man9/vgone.9 (working copy) @@ -26,24 +26,21 @@ .\ .\ $FreeBSD$ .\ -.Dd November 21, 2001 +.Dd October 27, 2010 .Dt VGONE 9 .Os .Sh NAME -.Nm vgone , vgonel +.Nm vgone .Nd prepare a vnode for reuse .Sh SYNOPSIS .In sys/param.h .In sys/vnode.h .Ft void .Fn vgone struct vnode *vp -.Ft void -.Fn vgonel struct vnode *vp struct thread *td .Sh DESCRIPTION +The .Fn vgone -and -.Fn vgonel -prepare a vnode for reuse by another file system. +function prepares a vnode for reuse by another file system. I think it is more correct now prepares the vnode to be destroyed instead. The preparation includes the cleaning of all file system specific data and the removal from its mount point vnode list. .Pp pgp1GMfQw2oLz.pgp Description: PGP signature
Re: [PATCH] Fix 'implicit declaration' warning and update vgone(9)
On Wed, 27 Oct 2010, John Baldwin wrote: On Wednesday, October 27, 2010 7:33:13 am Sergey Kandaurov wrote: On 27 October 2010 10:23, Lars Hartmann l...@chaotika.org wrote: The vgonel function isnt declarated in any header, the vgonel prototype in vgone(9) isnt correct - found by Ben Kaduk ka...@mit.edu Hi. I'm afraid it's just an overlooked man page after many VFS changes in 5.x. As vgonel() is a static (i.e. private and not visible from outside) function IMO it should be removed from vgone(9) man page. Agreed. It certainly should not be added to vnode.h. I'm curious how the reporter is getting a warning since there is a static prototype for vgonel() in vfs_subr.c. It's for a third-party kernel module, for the OpenAFS filesystem (which has been unloved for FreeBSD since the 4.X days). The AFS code is currently using unlocked accesses to v_usecount (which, unsurprisingly, led to a race condition that caused an invariant check to fail), and I was going through and adding the interlock around them. At the place that I suspect to be the main cause of this race [1], the usecount was checked to be nonpositive along with a couple other conditions, and a little later vgone() was called. Holding the interlock across both of these calls (and therefore using vgonel()) seems to have closed the race condition I was seeing. (Other checks of v_usecount were changed to grab the interlock, but drop it before doing anything else.) However, looking at the commit message for vfs_subr.c:1.631, I guess this is not the locking strategy I'm supposed to be using? I saw a warning of implicit declaration when compiling the kernel module, but the kernel linker was happy to load it. I forget whether it matters that vgonel is only declared static at its declaration but not its definition. -Ben Kaduk [1] The old (racy) function is osi_TryEvictVCache, here: http://git.openafs.org/?p=openafs.git;a=blob;f=src/afs/FBSD/osi_vcache.c;h=c2060c74f0155a610d2ea94f3c7f508e8ca4373a;hb=HEAD ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: dump cache performance
Peter Jeremy peterjer...@acm.org writes: I've mostly convered to ZFS but still have UFS root (which is basically a full base install without /var but including /usr/src - 94k inodes and 1.7GB). I've run both the 8-stable (stable) and patched (jfd) dump alternately 4 times with 50/250MB cache with the following results: x stable + jfd ++ | +| | +| | x+| |x xx +| ||AMA| ++ N Min MaxMedian AvgStddev x 4 9413 9673 95689555.5 107.12143 + 4 15359 15359 15359 15359 0 9413 what? Puppies? DES -- Dag-Erling Smørgrav - d...@des.no ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: dump cache performance
On 2010-Oct-27 20:17:06 +0200, Dag-Erling Smørgrav d...@des.no wrote: Peter Jeremy peterjer...@acm.org writes: I've mostly convered to ZFS but still have UFS root (which is basically a full base install without /var but including /usr/src - 94k inodes and 1.7GB). I've run both the 8-stable (stable) and patched (jfd) dump alternately 4 times with 50/250MB cache with the following results: x stable + jfd ++ | +| | +| | x+| |x xx +| ||AMA| ++ N Min MaxMedian AvgStddev x 4 9413 9673 95689555.5 107.12143 + 4 15359 15359 15359 15359 0 9413 what? Puppies? Ooops, sorry - KB/sec as reported in the dump summary. -- Peter Jeremy pgp3QZs1Cb6ob.pgp Description: PGP signature
Re: dump cache performance
Peter Jeremy peterjer...@acm.org writes: Dag-Erling Smørgrav d...@des.no wrote: 9413 what? Puppies? Ooops, sorry - KB/sec as reported in the dump summary. Thank you :) DES -- Dag-Erling Smørgrav - d...@des.no ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Contiguous physical memory
Can anyone suggest a method/formula to monitor contiguous physical memory allocations such that one could predict when contigmalloc(), make that bus_dmamem_alloc might fail? Dr ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: Contiguous physical memory
On Wed, Oct 27, 2010 at 2:17 PM, Dr. Baud drb...@yahoo.com wrote: Can anyone suggest a method/formula to monitor contiguous physical memory allocations such that one could predict when contigmalloc(), make that bus_dmamem_alloc might fail? From the command line you can obtain this information with sysctl vm.phys_free. Alan ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org
Re: [PATCH] Fix 'implicit declaration' warning and update vgone(9)
On Wed, 27 Oct 2010, Kostik Belousov wrote: On Wed, Oct 27, 2010 at 10:59:56AM -0400, Benjamin Kaduk wrote: [1] The old (racy) function is osi_TryEvictVCache, here: http://git.openafs.org/?p=openafs.git;a=blob;f=src/afs/FBSD/osi_vcache.c;h=c2060c74f0155a610d2ea94f3c7f508e8ca4373a;hb=HEAD The function looks very strange for much more serious reasons. Why do you try to manage the vnode revocation in the filesystem module at all ? I am still becoming familiar with the AFS code, but I think this is largely due to a difference in the vfs structure that AFS has been using and the FreeBSD standard. E.g. vop_inactive/vop_reclaim do not actually free filesystem-specific resources, instead keeping a free list of vcache entries. So, the original authors of this AFS code were approaching the problem in a somewhat different way. Therefore, are somewhat-orthogonal pools of vcaches and vnodes (with some intersection). If the vcaches are all in use in use, there is a routine which tries to shake some loose; if it can free up vcaches, their associated vnodes also need to be cleaned up in some fashion. It may be that no additional code is actually needed to do this, though -- I am not sure. VFS will (assumedly in a right way) revoke and destroy non-referenced vnodes when needed. You are, in essence, suggesting that the body of the conditional should be just return 1;? (In my incomplete testing the vref() call was always made.) Anyway, you need to hold vnode lock before checking for the vnode refcounter. See the vfs_subr.c:vlrureclaim() for the correct dance with vholdl()/vn_lock() sequence that does the revocation I mentioned in the race-free way. Unless I am mistaken, the path you refer to still uses vgonel(), which is not available to me. Is it safe to drop and reaquire the interlock between the vgone and vdrop? Many thanks, Ben ___ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org