[PATCH] Fix 'implicit declaration' warning and update vgone(9)

2010-10-27 Thread Lars Hartmann
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)

2010-10-27 Thread Lars Hartmann
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

2010-10-27 Thread perryh
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

2010-10-27 Thread Bruce Cran
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

2010-10-27 Thread Ivan Voras
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

2010-10-27 Thread Sergey Kandaurov
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)

2010-10-27 Thread Sergey Kandaurov
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

2010-10-27 Thread Alexander Best
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

2010-10-27 Thread Garrett Cooper
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

2010-10-27 Thread Alexander Best
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

2010-10-27 Thread Garrett Cooper
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

2010-10-27 Thread Oliver Fromme

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

2010-10-27 Thread Oliver Fromme

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)

2010-10-27 Thread Sergey Kandaurov
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)

2010-10-27 Thread John Baldwin
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

2010-10-27 Thread Alexander Motin
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

2010-10-27 Thread Bruce Cran
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)

2010-10-27 Thread Kostik Belousov
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)

2010-10-27 Thread Benjamin Kaduk

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

2010-10-27 Thread Dag-Erling Smørgrav
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

2010-10-27 Thread Peter Jeremy
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

2010-10-27 Thread Dag-Erling Smørgrav
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

2010-10-27 Thread Dr. Baud

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

2010-10-27 Thread Alan Cox
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)

2010-10-27 Thread Benjamin Kaduk

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