Re: patch #3 Re: swapoff code comitted.
The swapctl code has been comitted. Bruce, you never supplied feedback in regards to your original nits, but feel free to clean the code up now that it has been comitted. All other bullets have been taken care of. I'm still on the fence in regards to backporting it. I would like to, but do not have the time at the moment. -Matt To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: patch #3 Re: swapoff code comitted.
[Reply to old mail] On Wed, 18 Dec 2002, Matthew Dillon wrote: :Looks good to me, modulo a few nits. I try not to nitpick, but :I've mentioned a few of them below. (BDE does a better job of it :than I do anyway. :-) : :The patch puts identical functionality in two places, so maybe it :would make sense to rip support for -s out of pstat/swapinfo (and :integrate 'pstat -ss' support into swapctl). If we really want to :go the NetBSD way, we could even integrate the swapon(2) and :swapoff(2) into swapctl(2). Doesn't matter to me. I think we should keep swapon and swapoff as separate commands. They are the most intuitive of the lot. Fine with me. I think I'd prefer them to be in separate source files too. That would be cleaner and I would find it easier to not see the style bugs in the new code. NetBSD's pstat supports -s, as does OpenBSD's, so there is no reason to rip out support for -s in our pstat. Neither OpenBSD or NetBSD have swapinfo that I can find. We could potentially rip out the swapinfo command though all it is is a hardlink to pstat so it wouldn't really save us anything. swapinfo is compatibility cruft for 386BSD-0.x through FreeBSD-1.x. pstat -s is the tradional place for this, and the other BSDs apparently dropped the compatibility cruft when they started using it. swapinfo is hard to drop now that it has been in FreeBSD for so long. I'd prefer not to have swapctl. It is just compatibibility cruft for NetBSD and OpenBSD. We have nothing interesting to control except on and off, which are more intuitively controlled by swapon and swapoff. :[...] : + if (strstr(argv[0], swapon)) : + which_prog = SWAPON; : + else if (strstr(argv[0], swapoff)) : + which_prog = SWAPOFF; : :It's probably better to do a strcmp on strrchr(argv[0], '/') when :argv[0] contains a slash. Otherwise people will wonder why :swapoff(8) breaks when they (perhaps mistakenly) compile and run :it from the src/sbin/swapon directory. Hmm. How about a strstr on a strrchr. I don't like making exact comparisons because it removes flexibility that someone might want in regards to hardlinks (for example, someone might want to add a version or other suffix to differentiate certain binaries in a test environment or in an emulation environment). e.g. bsdswapon vs swapon. I disagree with having this flexibility in programs whose behaviour depends on their name. Isn't there a shortcut procedure to handle the NULL return case? I know there is one for a forward scan. I thought there was one for the reverse scan too. if ((ptr = strrchr(argv[0], '/')) == NULL) ptr = argv[0]; if (strstr(ptr, swapon)) ... Isn't basename(3) supposed to be used for things like this? Examples of existing practice: chown.c: - Uses home made basename. - Uses strcmp() with chown, so the program acts as chown(8) if its basename is precisely chown and as chgrp(8) otherwise. - The strcmp() statement is missing style bugs. rmextattr.c: - Uses basename(3). - Uses strcmp() with the 4 valid variants and a usage message if no match, so the program name must match precisely. - The strcmp() statements use the style bug !strcmp(). pstat.c: - Like chown.c except the strcmp() statement uses the style bug !strcmp(). Bruce To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: patch #3 Re: swapoff code comitted.
On Sat, 28 Dec 2002, Matthew Dillon wrote: The swapctl code has been comitted. Bruce, you never supplied feedback in regards to your original nits, but feel free to clean the code up now that it has been comitted. All other bullets have been taken care of. I'm not sure if I want to get that involved. It has enough nits to form a few gnats. I most recently noticed misindented case statements and printf format errors which generate mail from non-i386 tinderboxes. Bruce To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: patch #3 Re: swapoff code comitted.
Thus spake Matthew Dillon [EMAIL PROTECTED]: :Looks good to me, modulo a few nits. I try not to nitpick, but :I've mentioned a few of them below. (BDE does a better job of it :than I do anyway. :-) : :The patch puts identical functionality in two places, so maybe it :would make sense to rip support for -s out of pstat/swapinfo (and :integrate 'pstat -ss' support into swapctl). If we really want to :go the NetBSD way, we could even integrate the swapon(2) and :swapoff(2) into swapctl(2). Doesn't matter to me. I think we should keep swapon and swapoff as separate commands. They are the most intuitive of the lot. NetBSD's pstat supports -s, as does OpenBSD's, so there is no reason to rip out support for -s in our pstat. Neither OpenBSD or NetBSD have swapinfo that I can find. We could potentially rip out the swapinfo command though all it is is a hardlink to pstat so it wouldn't really save us anything. I guess I'm just bothered by the fact that it's duplicating functionality. (In particular, the part that is duplicated was working fine in pstat and doesn't need to be on the root filesystem.) But when it comes down to it, I don't have a problem as long as other people are maintaining it. : + if (strstr(argv[0], swapon)) : + which_prog = SWAPON; : + else if (strstr(argv[0], swapoff)) : + which_prog = SWAPOFF; : :It's probably better to do a strcmp on strrchr(argv[0], '/') when :argv[0] contains a slash. Otherwise people will wonder why :swapoff(8) breaks when they (perhaps mistakenly) compile and run :it from the src/sbin/swapon directory. Hmm. How about a strstr on a strrchr. I don't like making exact comparisons because it removes flexibility that someone might want in regards to hardlinks (for example, someone might want to add a version or other suffix to differentiate certain binaries in a test environment or in an emulation environment). e.g. bsdswapon vs swapon. Isn't there a shortcut procedure to handle the NULL return case? I know there is one for a forward scan. I thought there was one for the reverse scan too. if ((ptr = strrchr(argv[0], '/')) == NULL) ptr = argv[0]; if (strstr(ptr, swapon)) ... Sounds fine. I don't know of a more concise approach offhand, and the original version used essentially what you just wrote. (I used strcmp(), so ptr had to be incremented to skip the slash.) :The repeated 'whichprog == foo' tests can be combined into a :single test at the end of the loop. They do subtly different things so I am not sure what you mean. You need to provide some code here. Yow, I didn't realize that -a and -d mean completely different things in swalctl vs swapon/swapoff. If that's how it has to work for compatibility, then it looks okay to me. To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: swapoff code comitted.
On Sun, Dec 15, 2002 at 02:47:51PM -0800, Matthew Dillon wrote:
:
:How about renaming swapon(8) into swapctl(8) after this function
enhancemen=
:t?
:This name reflects it's purpose much better and would be consistent
with the
:other BSDs.
:
:- Christian
I am not volunteering to do this, at least not right now. I
have too
big a stack of things that still need to be committed, but if
someone
else would like to tackle this I think it would be a nice little
project
for a developer with some free time to waste and I would be
happy to
review and test the work.
I have made a small patch, added l, s and h switches to show
information about the swap devices. And the U switch to swapctl only
to remove all activated swap devices.
If anything else is needed let me know and I will add it.
--
Eirik Nygaard [EMAIL PROTECTED]
PGP Key: 83C55EDE
Index: sbin/swapon/Makefile
===
RCS file: /home/ncvs/src/sbin/swapon/Makefile,v
retrieving revision 1.7
diff -u -r1.7 Makefile
--- sbin/swapon/Makefile15 Dec 2002 19:17:56 - 1.7
+++ sbin/swapon/Makefile17 Dec 2002 17:00:47 -
@@ -3,7 +3,9 @@
PROG= swapon
MAN= swapon.8
-LINKS= ${BINDIR}/swapon ${BINDIR}/swapoff
+LINKS= ${BINDIR}/swapoff ${BINDIR}/swapon
+LINKS+=${BINDIR}/swapctl ${BINDIR}/swapon
MLINKS=swapon.8 swapoff.8
+LDADD= -lc -lkvm
.include bsd.prog.mk
Index: sbin/swapon/swapon.c
===
RCS file: /home/ncvs/src/sbin/swapon/swapon.c,v
retrieving revision 1.13
diff -u -r1.13 swapon.c
--- sbin/swapon/swapon.c15 Dec 2002 19:17:56 - 1.13
+++ sbin/swapon/swapon.c17 Dec 2002 17:00:47 -
@@ -52,10 +52,17 @@
#include stdlib.h
#include string.h
#include unistd.h
+#include kvm.h
+#include fcntl.h
-static void usage(const char *);
+#define MAXSWAP 100
+
+static void usage(void);
static int is_swapoff(const char *);
+static int is_swapon(const char *);
+static int is_swapctl(const char *);
intswap_on_off(char *name, int ignoreebusy, int do_swapoff);
+void swaplist(int, int, int);
int
main(int argc, char **argv)
@@ -63,41 +70,68 @@
struct fstab *fsp;
int stat;
int ch, doall;
- int do_swapoff;
- char *pname = argv[0];
-
- do_swapoff = is_swapoff(pname);
-
+ int do_swapoff, do_swapon;
+ int sflag = 0, lflag = 0, hflag = 0;
+
+ do_swapoff = is_swapoff(getprogname());
+ do_swapon = is_swapon(getprogname());
+
doall = 0;
- while ((ch = getopt(argc, argv, a)) != -1)
- switch((char)ch) {
+ while ((ch = getopt(argc, argv, alhsU)) != -1)
+ switch(ch) {
case 'a':
doall = 1;
break;
+ case 's':
+ sflag = 1;
+ break;
+ case 'l':
+ lflag = 1;
+ break;
+ case 'h':
+ hflag = 1;
+ break;
+ case 'U':
+ if (!do_swapon) {
+ doall = 1;
+ do_swapoff = 1;
+ break;
+ } /* Remove the if if you want the U switch to work with
+swapon also, don't know if that is wanted */
case '?':
default:
- usage(pname);
+ usage();
}
argv += optind;
-
+
stat = 0;
- if (doall)
- while ((fsp = getfsent()) != NULL) {
- if (strcmp(fsp-fs_type, FSTAB_SW))
- continue;
- if (strstr(fsp-fs_mntops, noauto))
- continue;
- if (swap_on_off(fsp-fs_spec, 1, do_swapoff))
- stat = 1;
- else
- printf(%s: %sing %s as swap device\n,
- pname, do_swapoff ? remov : add,
- fsp-fs_spec);
+ if (do_swapoff || do_swapon) {
+ if (doall) {
+ while ((fsp = getfsent()) != NULL) {
+ if (strcmp(fsp-fs_type, FSTAB_SW))
+ continue;
+ if (strstr(fsp-fs_mntops, noauto))
+ continue;
+ if (swap_on_off(fsp-fs_spec, 1, do_swapoff))
+ stat = 1;
+ else
+ printf(%s: %sing %s as swap device\n,
+ getprogname(), do_swapoff ? remov :
Re: swapoff code comitted.
:I have made a small patch, added l, s and h switches to show
:information about the swap devices. And the U switch to swapctl only
:to remove all activated swap devices.
:If anything else is needed let me know and I will add it.
:
:--=20
:
:Eirik Nygaard [EMAIL PROTECTED]
:PGP Key: 83C55EDE
That is a pretty good first attempt. I have a few suggests and found
one bug. First the bug:
:+ is_swapctl ? lsU : );
I think that was supposed to be a call to is_swapctl, not a pointer
to the function.
Suggestions: Get rid of the is_swap*() functions and instead use
av[0] at the top of main() and use strstr() to determine if the
program is swapon, swapoff, or swapctl. Check against swapon and
swapoff and if it is neither then default to swapctl (don't test
against swapctl). Store which program it is in a global variable,
e.g. an enum like this:
enum { SWAPON, SWAPOFF, SWAPCTL } which_prog = SWAPCTL;
...
main(...)
{
if (strstr(av[0], swapon))
which_prog = SWAPON;
else if (strstr(av[0], swapoff))
which_prog = SWAPOFF;
...
}
In regards to retrieving swap information, in -current there is a
sysctl() to do it. Take a look at /usr/src/usr.sbin/pstat/pstat.c
(in the current source tree), at the swapmode_kvm() and swapmode_sysctl()
functions. The sysctl is much, much faster then the kvm call because
the kvm call has to run through the swap radix tree to collect the useage
information.
-Matt
Matthew Dillon
[EMAIL PROTECTED]
To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-current in the body of the message
Re: swapoff code comitted.
On Wed, Dec 18, 2002 at 11:18:24AM -0800, Matthew Dillon wrote:
:I have made a small patch, added l, s and h switches to show
:information about the swap devices. And the U switch to swapctl only
:to remove all activated swap devices.
:If anything else is needed let me know and I will add it.
:
:--=20
:
:Eirik Nygaard [EMAIL PROTECTED]
:PGP Key: 83C55EDE
That is a pretty good first attempt. I have a few suggests and found
one bug. First the bug:
:+is_swapctl ? lsU : );
I think that was supposed to be a call to is_swapctl, not a pointer
to the function.
Suggestions: Get rid of the is_swap*() functions and instead use
av[0] at the top of main() and use strstr() to determine if the
program is swapon, swapoff, or swapctl. Check against swapon and
swapoff and if it is neither then default to swapctl (don't test
against swapctl). Store which program it is in a global variable,
e.g. an enum like this:
enum { SWAPON, SWAPOFF, SWAPCTL } which_prog = SWAPCTL;
In regards to retrieving swap information, in -current there is a
sysctl() to do it. Take a look at /usr/src/usr.sbin/pstat/pstat.c
(in the current source tree), at the swapmode_kvm() and swapmode_sysctl()
functions. The sysctl is much, much faster then the kvm call because
the kvm call has to run through the swap radix tree to collect the useage
information.
Added the enum instead of is_swap* commands and changed from kvm to
sysctl to get the swap information.
--
Eirik Nygaard [EMAIL PROTECTED]
PGP Key: 83C55EDE
? sbin/swapon/.swapon.c.swp
Index: sbin/swapon/Makefile
===
RCS file: /home/ncvs/src/sbin/swapon/Makefile,v
retrieving revision 1.7
diff -u -r1.7 Makefile
--- sbin/swapon/Makefile15 Dec 2002 19:17:56 - 1.7
+++ sbin/swapon/Makefile18 Dec 2002 20:35:53 -
@@ -3,7 +3,9 @@
PROG= swapon
MAN= swapon.8
-LINKS= ${BINDIR}/swapon ${BINDIR}/swapoff
-MLINKS=swapon.8 swapoff.8
+LINKS= ${BINDIR}/swapoff ${BINDIR}/swapon
+LINKS+=${BINDIR}/swapctl ${BINDIR}/swapon
+MLINKS=swapoff.8 swapon.8
+MLINKS=swapctl.8 swapon.8
.include bsd.prog.mk
Index: sbin/swapon/swapon.c
===
RCS file: /home/ncvs/src/sbin/swapon/swapon.c,v
retrieving revision 1.13
diff -u -r1.13 swapon.c
--- sbin/swapon/swapon.c15 Dec 2002 19:17:56 - 1.13
+++ sbin/swapon/swapon.c18 Dec 2002 20:35:53 -
@@ -45,6 +45,11 @@
$FreeBSD: src/sbin/swapon/swapon.c,v 1.13 2002/12/15 19:17:56 dillon Exp $;
#endif /* not lint */
+#include sys/stat.h
+#include sys/param.h
+#include sys/user.h
+#include sys/sysctl.h
+
#include err.h
#include errno.h
#include fstab.h
@@ -52,10 +57,13 @@
#include stdlib.h
#include string.h
#include unistd.h
+#include fcntl.h
+
+static void usage(void);
+intswap_on_off(char *name, int ignoreebusy);
+void swaplist(int, int, int);
-static void usage(const char *);
-static int is_swapoff(const char *);
-intswap_on_off(char *name, int ignoreebusy, int do_swapoff);
+enum { SWAPON, SWAPOFF, SWAPCTL } which_prog = SWAPCTL;
int
main(int argc, char **argv)
@@ -63,47 +71,80 @@
struct fstab *fsp;
int stat;
int ch, doall;
- int do_swapoff;
- char *pname = argv[0];
-
- do_swapoff = is_swapoff(pname);
-
+ int sflag = 0, lflag = 0, hflag = 0;
+
+ if (strstr(argv[0], swapon))
+ which_prog = SWAPON;
+ else if (strstr(argv[0], swapoff))
+ which_prog = SWAPOFF;
+
doall = 0;
- while ((ch = getopt(argc, argv, a)) != -1)
- switch((char)ch) {
+ while ((ch = getopt(argc, argv, alhsU)) != -1)
+ switch(ch) {
case 'a':
doall = 1;
break;
+ case 's':
+ sflag = 1;
+ break;
+ case 'l':
+ lflag = 1;
+ break;
+ case 'h':
+ hflag = 1;
+ break;
+ case 'U':
+ if (which_prog != SWAPON) {
+ doall = 1;
+ which_prog = SWAPOFF;
+ break;
+ } /* Remove the if if you want the U switch to work with
+swapon also, don't know if that is wanted */
case '?':
default:
- usage(pname);
+ usage();
}
argv += optind;
-
+
stat = 0;
- if (doall)
- while ((fsp = getfsent()) != NULL) {
- if (strcmp(fsp-fs_type, FSTAB_SW))
- continue;
-
Re: swapoff code comitted.
:Added the enum instead of is_swap* commands and changed from kvm to
:sysctl to get the swap information.
:
:Eirik Nygaard [EMAIL PROTECTED]
:PGP Key: 83C55EDE
All right, I found a couple more bugs and fleshed it out a bit.
You got your LINKS and MLINKS reversed and forgot a +=,
you forgot to initialize the do_swapoff variable in swap_on_off(),
and you reuse 'total' and 'used' in swaplist() in a manner which breaks
the -s option.
I have included an updated patch below based on these fixes and a few
other minor cleanups. I also changed the block size for -h from 1000
to 1024 bytes to make it consistent with pstat -s and friends (and
also OpenBSD and NetBSD).
I also added -A, -U, cleaned up usage(), and made the options conform
to NetBSD and OpenBSD.
The only thing really missing is for us to handle the BLOCKSIZE
environment variable like 'df' does, and appropriate additions to
the manual page (which I would be happy to do). Once we get those in
I will commit it.
Here is an updated patch.
-Matt
Index: Makefile
===
RCS file: /home/ncvs/src/sbin/swapon/Makefile,v
retrieving revision 1.7
diff -u -r1.7 Makefile
--- Makefile15 Dec 2002 19:17:56 - 1.7
+++ Makefile18 Dec 2002 21:31:41 -
@@ -4,6 +4,8 @@
PROG= swapon
MAN= swapon.8
LINKS= ${BINDIR}/swapon ${BINDIR}/swapoff
+LINKS+=${BINDIR}/swapon ${BINDIR}/swapctl
MLINKS=swapon.8 swapoff.8
+MLINKS+=swapon.8 swapctl.8
.include bsd.prog.mk
Index: swapon.c
===
RCS file: /home/ncvs/src/sbin/swapon/swapon.c,v
retrieving revision 1.13
diff -u -r1.13 swapon.c
--- swapon.c15 Dec 2002 19:17:56 - 1.13
+++ swapon.c18 Dec 2002 22:20:42 -
@@ -45,6 +45,11 @@
$FreeBSD: src/sbin/swapon/swapon.c,v 1.13 2002/12/15 19:17:56 dillon Exp $;
#endif /* not lint */
+#include sys/stat.h
+#include sys/param.h
+#include sys/user.h
+#include sys/sysctl.h
+
#include err.h
#include errno.h
#include fstab.h
@@ -52,10 +57,13 @@
#include stdlib.h
#include string.h
#include unistd.h
+#include fcntl.h
+
+static void usage(void);
+static int swap_on_off(char *name, int ignoreebusy);
+static void swaplist(int, int, int);
-static void usage(const char *);
-static int is_swapoff(const char *);
-intswap_on_off(char *name, int ignoreebusy, int do_swapoff);
+enum { SWAPON, SWAPOFF, SWAPCTL } orig_prog, which_prog = SWAPCTL;
int
main(int argc, char **argv)
@@ -63,48 +71,105 @@
struct fstab *fsp;
int stat;
int ch, doall;
- int do_swapoff;
- char *pname = argv[0];
-
- do_swapoff = is_swapoff(pname);
-
+ int sflag = 0, lflag = 0, hflag = 0;
+
+ if (strstr(argv[0], swapon))
+ which_prog = SWAPON;
+ else if (strstr(argv[0], swapoff))
+ which_prog = SWAPOFF;
+ orig_prog = which_prog;
+
doall = 0;
- while ((ch = getopt(argc, argv, a)) != -1)
- switch((char)ch) {
+ while ((ch = getopt(argc, argv, AadlhksU)) != -1) {
+ switch(ch) {
+ case 'A':
+ if (which_prog == SWAPCTL) {
+ doall = 1;
+ which_prog = SWAPON;
+ } else {
+ usage();
+ }
+ break;
case 'a':
- doall = 1;
+ if (which_prog == SWAPON || which_prog == SWAPOFF)
+ doall = 1;
+ else
+ which_prog = SWAPON;
+ break;
+ case 'd':
+ if (which_prog == SWAPCTL)
+ which_prog = SWAPOFF;
+ else
+ usage();
+ break;
+ case 's':
+ sflag = 1;
+ break;
+ case 'l':
+ lflag = 1;
+ break;
+ case 'h':
+ hflag = 'M';
+ break;
+ case 'k':
+ hflag = 'K';
+ break;
+ case 'U':
+ if (which_prog == SWAPCTL) {
+ doall = 1;
+ which_prog = SWAPOFF;
+ } else {
+ usage();
+ }
break;
case '?':
default:
- usage(pname);
+ usage();
}
+ }
argv += optind;
-
+
stat = 0;
- if (doall)
-
patch #3 Re: swapoff code comitted.
Here's another update. I cleaned things up even more, add BLOCKSIZE
support, and updated the manual page. It looks quite nice now.
-Matt
Index: Makefile
===
RCS file: /home/ncvs/src/sbin/swapon/Makefile,v
retrieving revision 1.7
diff -u -r1.7 Makefile
--- Makefile15 Dec 2002 19:17:56 - 1.7
+++ Makefile18 Dec 2002 21:31:41 -
@@ -4,6 +4,8 @@
PROG= swapon
MAN= swapon.8
LINKS= ${BINDIR}/swapon ${BINDIR}/swapoff
+LINKS+=${BINDIR}/swapon ${BINDIR}/swapctl
MLINKS=swapon.8 swapoff.8
+MLINKS+=swapon.8 swapctl.8
.include bsd.prog.mk
Index: swapon.8
===
RCS file: /home/ncvs/src/sbin/swapon/swapon.8,v
retrieving revision 1.21
diff -u -r1.21 swapon.8
--- swapon.815 Dec 2002 19:17:56 - 1.21
+++ swapon.818 Dec 2002 22:46:01 -
@@ -43,45 +43,101 @@
.Fl a
.Nm swap[on|off]
.Ar special_file ...
+.Nm swapctl
+.Fl lshk
+.Nm swapctl
+.Fl AU
+.Nm swapctl
+.Fl a
+.Ar special_file ...
+.Nm swapctl
+.Fl d
+.Ar special_file ...
.Sh DESCRIPTION
The
+.Nm swap[on,off,ctl]
+utilties are used to control swap devices in the system. At boot time all
+swap entries in
+.Pa /etc/fstab
+are added automatically when the system goes multi-user.
+Swap devices are interleaved and kernels are typically configured
+to handle a maximum of 4 swap devices. There is no priority mechanism.
+.Pp
+The
.Nm swapon
-utility is used to specify additional devices on which paging and swapping
-are to take place.
-The system begins by swapping and paging on only a single device
-so that only one disk is required at bootstrap time.
-Calls to
-.Nm swapon
-normally occur in the system multi-user initialization file
-.Pa /etc/rc
-making all swap devices available, so that the paging and swapping
-activity is interleaved across several devices.
+utility adds the specified swap devices to the system. If the
+.Fl a
+option is used, all swap devices in
+.Pa /etc/fstab
+will be added, unless their ``noauto'' option is also set.
.Pp
The
.Nm swapoff
-utility disables paging and swapping on a device.
-Calls to
+utility removes the specified swap devices from the system. If the
+.Fl a
+option is used, all swap devices in
+.Pa /etc/fstab
+will be removed, unless their ``noauto'' option is also set.
+Note that
.Nm swapoff
-succeed only if disabling the device would leave enough
-remaining virtual memory to accomodate all running programs.
+will fail and refuse to remove a swap device if there is insufficient
+VM (memory + remaining swap devices) to run the system.
+.Nm Swapoff
+must move sawpped pages out of the device being removed which could
+lead to high system loads for a period of time, depending on how
+much data has been swapped out to that device.
.Pp
-Normally, the first form is used:
-.Bl -tag -width indent
-.It Fl a
-All devices marked as ``sw''
-swap devices in
+The
+.Nm swapctl
+utility exists primarily for those familiar with other BSDs and may be
+used to add, remove, or list swap. Note that the
+.Fl a
+option is used diferently in
+.Nm swapctl
+and indicates that a specific list of devices should be added.
+The
+.Fl d
+option indicates that a specific list should be removed. The
+.Fl A
+and
+.Fl D
+options to
+.Nm swapctl
+operate on all swap entries in
.Pa /etc/fstab
-are added to or removed from the pool of available swap
-unless their ``noauto'' option is also set.
-.El
+which do not have their ``noauto'' option set.
+.Pp
+Swap information can be generated using the
+.Nm swapinfo
+program,
+.Nm pstat
+.Fl s ,
+or
+.Nm swapctl
+.Fl lshk .
+The
+.Nm swapctl
+utility has the following options for listing swap:
+.Bl -tag -width indent
+.It Fl l
+List the devices making up system swap.
+.It Fl s
+Print a summary line for system swap.
+.It Fl h
+Output values in megabytes.
+.It Fl k
+Output values in kilobytes.
.Pp
-The second form is used to configure or disable individual devices.
+The BLOCKSIZE environment variable is used if not specifically
+overridden. 512 byte blocks are used by default.
+.El
.Sh SEE ALSO
.Xr swapon 2 ,
.Xr fstab 5 ,
.Xr init 8 ,
.Xr mdconfig 8 ,
.Xr pstat 8 ,
+.Xr swapinfo 8 ,
.Xr rc 8
.Sh FILES
.Bl -tag -width /dev/{ad,da}?s?b -compact
Index: swapon.c
===
RCS file: /home/ncvs/src/sbin/swapon/swapon.c,v
retrieving revision 1.13
diff -u -r1.13 swapon.c
--- swapon.c15 Dec 2002 19:17:56 - 1.13
+++ swapon.c18 Dec 2002 22:53:52 -
@@ -45,6 +45,11 @@
$FreeBSD: src/sbin/swapon/swapon.c,v 1.13 2002/12/15 19:17:56 dillon Exp $;
#endif /* not lint */
+#include sys/stat.h
+#include sys/param.h
+#include sys/user.h
+#include sys/sysctl.h
+
#include err.h
#include errno.h
#include fstab.h
@@ -52,10 +57,13 @@
#include stdlib.h
#include string.h
#include unistd.h
+#include
Re: patch #3 Re: swapoff code comitted.
:Looks good to me, modulo a few nits. I try not to nitpick, but
:I've mentioned a few of them below. (BDE does a better job of it
:than I do anyway. :-)
:
:The patch puts identical functionality in two places, so maybe it
:would make sense to rip support for -s out of pstat/swapinfo (and
:integrate 'pstat -ss' support into swapctl). If we really want to
:go the NetBSD way, we could even integrate the swapon(2) and
:swapoff(2) into swapctl(2). Doesn't matter to me.
I think we should keep swapon and swapoff as separate commands.
They are the most intuitive of the lot.
NetBSD's pstat supports -s, as does OpenBSD's, so there is no reason
to rip out support for -s in our pstat.
Neither OpenBSD or NetBSD have swapinfo that I can find. We could
potentially rip out the swapinfo command though all it is is a hardlink
to pstat so it wouldn't really save us anything.
:(BTW, when I get the chance, I'll re-run my swapoff torture tests
:now that Alan Cox's new locking is in place. Chances are the
:swapoff code needs to be brought up to date..)
I ran it across Alan and he thought it looked ok at a glance, but
I agree now that it is integrated in we should take a more involved
look at it.
:...
:[...]
: +if (strstr(argv[0], swapon))
: +which_prog = SWAPON;
: +else if (strstr(argv[0], swapoff))
: +which_prog = SWAPOFF;
:
:It's probably better to do a strcmp on strrchr(argv[0], '/') when
:argv[0] contains a slash. Otherwise people will wonder why
:swapoff(8) breaks when they (perhaps mistakenly) compile and run
:it from the src/sbin/swapon directory.
Hmm. How about a strstr on a strrchr. I don't like making exact
comparisons because it removes flexibility that someone might want
in regards to hardlinks (for example, someone might want to add a
version or other suffix to differentiate certain binaries in a test
environment or in an emulation environment). e.g. bsdswapon vs
swapon.
Isn't there a shortcut procedure to handle the NULL return case?
I know there is one for a forward scan. I thought there was one for
the reverse scan too.
if ((ptr = strrchr(argv[0], '/')) == NULL)
ptr = argv[0];
if (strstr(ptr, swapon))
...
: +if (which_prog == SWAPCTL) {
: +doall = 1;
: +which_prog = SWAPON;
: +} else {
: +usage();
: +}
: +break;
:[...]
:
:The repeated 'whichprog == foo' tests can be combined into a
:single test at the end of the loop.
They do subtly different things so I am not sure what you mean.
You need to provide some code here.
: -
: +
:
:?
It's probably a space or a tab. I'll track it down.
-Matt
Matthew Dillon
[EMAIL PROTECTED]
To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-current in the body of the message
Re: patch #3 Re: swapoff code comitted.
: :On Wed, 18 Dec 2002, Matthew Dillon wrote: : : Here's another update. I cleaned things up even more, add BLOCKSIZE : support, and updated the manual page. It looks quite nice now. : :I still dislike it. It starts by adding style bugs to the Makefile :(changing = to += for the initial assignments to LINKS and MLINKS) :and doesn't get any better. : :Bruce It hasn't been committed yet so please feel free to email me patches for stylistic issues over the next day or two. In regards to having a swapctl command at all, I think anything that offers familiarity between the BSDs is a good thing to have. If anything we should remove 'swapinfo'. 'pstat -s' is actually shorter and easier to type :-) -Matt Matthew Dillon [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: swapoff code comitted.
Thus spake Christian Brueffer [EMAIL PROTECTED]: How about renaming swapon(8) into swapctl(8) after this function enhancement? This name reflects it's purpose much better and would be consistent with the other BSDs. It would be trivial to change the name, although I don't see what it buys you. NetBSD changed the name because they wanted to be able to set swap device priority and do other things through the same interface. From previous conversations, I gather that swap device priority is not a particularly desired feature for FreeBSD, and it would probably require rewriting the entire swap subsystem again in any case. Unless you intend to extend the interface further, having swapctl seems to me like changing mount/umount to mountctl. That's not to say I'm opposed to the idea, just that I don't care one way or the other. To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
swapoff code comitted.
David Schultz's swapoff code has been comitted. It should be regarded as being highly experimental (and it still needs to be vetted for VM locking changes and other recent changes in -current). A considerable amount of testing has been done already but -current is a moving target. I am tentitively planning on MFCing the code in a few weeks (some time after christmas) but it will depend on my schedule. If someone else wants to take on that work I will would be happy to act as a reviewer. -Matt Matthew Dillon [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: swapoff code comitted.
On Sun, Dec 15, 2002 at 11:46:55AM -0800, Matthew Dillon wrote: David Schultz's swapoff code has been comitted. It should be regarded as being highly experimental (and it still needs to be vetted for VM locking changes and other recent changes in -current). A considerable amount of testing has been done already but -current is a moving target. I am tentitively planning on MFCing the code in a few weeks (some time after christmas) but it will depend on my schedule. If someone else wants to take on that work I will would be happy to act as a reviewer. -Matt Matthew Dillon [EMAIL PROTECTED] How about renaming swapon(8) into swapctl(8) after this function enhancement? This name reflects it's purpose much better and would be consistent with the other BSDs. - Christian -- http://www.unixpages.org[EMAIL PROTECTED] GPG Pub-Key: www.unixpages.org/cbrueffer.asc GPG Fingerprint: A5C8 2099 19FF AACA F41B B29B 6C76 178C A0ED 982D GPG Key ID : 0xA0ED982D msg48813/pgp0.pgp Description: PGP signature
Re: swapoff code comitted.
: :How about renaming swapon(8) into swapctl(8) after this function enhancemen= :t? :This name reflects it's purpose much better and would be consistent with the :other BSDs. : :- Christian I think that's an excellent idea. We would have to do some rewriting to add the expected options but it would not be too difficult to do. Mainly just grunt work. e.g. the NetBSD swapctl command is this: swapctl -A [-p priority] [-t blk|noblk] -D dumpdev -U [-t blk|noblk] -a [-p priority] path -c -p priority path -d path -l | -s [-k] -z swapon -a | path And the OpenBSD swapctl command is this: swapctl -A [-p priority] [-t blk|noblk] swapctl -a [-p priority] path swapctl -c -p priority path swapctl -d path swapctl -l | -s [-k] swapon -a | path We would simply ignore priority since we don't support it, nor would we need a -t option since we do not have buffered block devices any more or a -c option since, again, we do not have priorities. I would keep 'swapoff' in anycase. It's an obvious command and like swapon could simply wind up being a hardlink to swapctl. I am not volunteering to do this, at least not right now. I have too big a stack of things that still need to be committed, but if someone else would like to tackle this I think it would be a nice little project for a developer with some free time to waste and I would be happy to review and test the work. -Matt To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: swapoff code comitted.
On Sun, Dec 15, 2002 at 02:47:51PM -0800, Matthew Dillon wrote: : :How about renaming swapon(8) into swapctl(8) after this function enhancemen= :t? :This name reflects it's purpose much better and would be consistent with the :other BSDs. : :- Christian I think that's an excellent idea. We would have to do some rewriting to add the expected options but it would not be too difficult to do. Mainly just grunt work. e.g. the NetBSD swapctl command is this: swapctl -A [-p priority] [-t blk|noblk] -D dumpdev -U [-t blk|noblk] -a [-p priority] path -c -p priority path -d path -l | -s [-k] -z swapon -a | path And the OpenBSD swapctl command is this: swapctl -A [-p priority] [-t blk|noblk] swapctl -a [-p priority] path swapctl -c -p priority path swapctl -d path swapctl -l | -s [-k] swapon -a | path We would simply ignore priority since we don't support it, nor would we need a -t option since we do not have buffered block devices any more or a -c option since, again, we do not have priorities. I would keep 'swapoff' in anycase. It's an obvious command and like swapon could simply wind up being a hardlink to swapctl. I am not volunteering to do this, at least not right now. I have too big a stack of things that still need to be committed, but if someone else would like to tackle this I think it would be a nice little project for a developer with some free time to waste and I would be happy to review and test the work. -Matt I'll look into that when some of my current work is done. Maybe it's worth an entry in PHK's JKH TODO list for now. - Christian -- http://www.unixpages.org[EMAIL PROTECTED] GPG Pub-Key: www.unixpages.org/cbrueffer.asc GPG Fingerprint: A5C8 2099 19FF AACA F41B B29B 6C76 178C A0ED 982D GPG Key ID : 0xA0ED982D msg48830/pgp0.pgp Description: PGP signature
