Date: Sun, 31 Jul 2011 22:30:17 +0300 From: Kostik Belousov <kostikbel@gmail.com> To: Robert Millan <rmh@debian.org> Cc: freebsd-hackers@freebsd.org, freebsd-emulation@freebsd.org, Ed Maste <emaste@freebsd.org>, bde@freebsd.org Subject: Re: kern/159281: [PATCH] Linux-like /proc/swaps for linprocfs Message-ID: <20110731193017.GR17489@deviant.kiev.zoral.com.ua> In-Reply-To: <CAOfDtXMh=LBb_PH1H7e02SRU=HRHLPjhZ%2BxVKLTLiee0JQ3yfw@mail.gmail.com> References: <CAOfDtXOJjOoQA8yNFPVdQRqCqr-Vc5nscMbgOLGLMuvTP9mp1w@mail.gmail.com> <20110730173239.GA17489@deviant.kiev.zoral.com.ua> <CAOfDtXMh=LBb_PH1H7e02SRU=HRHLPjhZ%2BxVKLTLiee0JQ3yfw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--bQyVjumYeq8yaMqA
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Sun, Jul 31, 2011 at 06:56:51PM +0200, Robert Millan wrote:
> 2011/7/30 Kostik Belousov <kostikbel@gmail.com>:
> > Second, my proposal contains a flaw. Namely, if some swap device was re=
moved
> > between calls to swap_info and swap_devname calls, we get mangled list.
>=20
> Ok, I see that you fixed this by unifying those functions.
Rather, by gathering both pieces of information without dropping the
protection of sw_dev_mtx.
>=20
> > The third problem, which is not fixed, and which I do not want to handl=
e,
> > is that the swap devices list can be changed between calls to swap_devn=
ame(),
> > changing device indexes and thus making the output mangled.
>=20
> You say you don't want to handle this, but AFAICT from the patch you
> already did. Or did I miss something?
Since sw_dev_mtx is dropped and reacquired between each iteration of the
loop, some swap device may be removed meantime. At least, the output
will show the same device twice.
Hm, the swapoff/swapon routines are half-protected by the Giant, taking
the Giant around the loop prevents the swap device list changes.
>=20
> > Should the swap device name be separated from 'unknown' word by
> > space or by tab ?
>=20
> Linux' /proc/swaps prints spaces after the first column and tabs for
> the rest. I think it's better to do the same just in case, but I
> don't think it's relevant either way.
Right, I added a comment to not have this broken by somebody in future.
>=20
> > I updated your patch, hopefully fixing the issues. Do you have comments
> > or objections ?
>=20
> No. Thanks for fixing those problems.
Below is the hopefully final patch after Bruce Evans' comments incorporated.
If nobody speaks, I will send this to re tomorrow.
diff --git a/sys/compat/linprocfs/linprocfs.c b/sys/compat/linprocfs/linpro=
cfs.c
index 692c5a3..8832d3d 100644
--- a/sys/compat/linprocfs/linprocfs.c
+++ b/sys/compat/linprocfs/linprocfs.c
@@ -502,6 +502,33 @@ linprocfs_dostat(PFS_FILL_ARGS)
return (0);
}
=20
+static int
+linprocfs_doswaps(PFS_FILL_ARGS)
+{
+ struct xswdev xsw;
+ uintmax_t total, used;
+ int n;
+ char devname[SPECNAMELEN + 1];
+
+ sbuf_printf(sb, "Filename\t\t\t\tType\t\tSize\tUsed\tPriority\n");
+ mtx_lock(&Giant);
+ for (n =3D 0; ; n++) {
+ if (swap_dev_info(n, &xsw, devname, sizeof(devname)) !=3D 0)
+ break;
+ total =3D (uintmax_t)xsw.xsw_nblks * PAGE_SIZE / 1024;
+ used =3D (uintmax_t)xsw.xsw_used * PAGE_SIZE / 1024;
+
+ /*
+ * The space and not tab after the device name is on
+ * purpose. Linux does so.
+ */
+ sbuf_printf(sb, "/dev/%-34s unknown\t\t%jd\t%jd\t-1\n",
+ devname, total, used);
+ }
+ mtx_unlock(&Giant);
+ return (0);
+}
+
/*
* Filler function for proc/uptime
*/
@@ -1490,6 +1517,8 @@ linprocfs_init(PFS_INIT_ARGS)
NULL, NULL, NULL, 0);
pfs_create_file(root, "stat", &linprocfs_dostat,
NULL, NULL, NULL, PFS_RD);
+ pfs_create_file(root, "swaps", &linprocfs_doswaps,
+ NULL, NULL, NULL, PFS_RD);
pfs_create_file(root, "uptime", &linprocfs_douptime,
NULL, NULL, NULL, PFS_RD);
pfs_create_file(root, "version", &linprocfs_doversion,
diff --git a/sys/vm/swap_pager.c b/sys/vm/swap_pager.c
index f421e4f..9a818e7 100644
--- a/sys/vm/swap_pager.c
+++ b/sys/vm/swap_pager.c
@@ -2365,35 +2365,53 @@ swap_pager_status(int *total, int *used)
mtx_unlock(&sw_dev_mtx);
}
=20
-static int
-sysctl_vm_swap_info(SYSCTL_HANDLER_ARGS)
+int
+swap_dev_info(int name, struct xswdev *xs, char *devname, size_t len)
{
- int *name =3D (int *)arg1;
- int error, n;
- struct xswdev xs;
struct swdevt *sp;
-
- if (arg2 !=3D 1) /* name length */
- return (EINVAL);
+ char *tmp_devname;
+ int error, n;
=20
n =3D 0;
+ error =3D ENOENT;
mtx_lock(&sw_dev_mtx);
TAILQ_FOREACH(sp, &swtailq, sw_list) {
- if (n =3D=3D *name) {
- mtx_unlock(&sw_dev_mtx);
- xs.xsw_version =3D XSWDEV_VERSION;
- xs.xsw_dev =3D sp->sw_dev;
- xs.xsw_flags =3D sp->sw_flags;
- xs.xsw_nblks =3D sp->sw_nblks;
- xs.xsw_used =3D sp->sw_used;
-
- error =3D SYSCTL_OUT(req, &xs, sizeof(xs));
- return (error);
+ if (n !=3D name) {
+ n++;
+ continue;
+ }
+ xs->xsw_version =3D XSWDEV_VERSION;
+ xs->xsw_dev =3D sp->sw_dev;
+ xs->xsw_flags =3D sp->sw_flags;
+ xs->xsw_nblks =3D sp->sw_nblks;
+ xs->xsw_used =3D sp->sw_used;
+ if (devname !=3D NULL) {
+ if (vn_isdisk(sp->sw_vp, NULL))
+ tmp_devname =3D sp->sw_vp->v_rdev->si_name;
+ else
+ tmp_devname =3D "[file]";
+ strncpy(devname, tmp_devname, len);
}
- n++;
+ error =3D 0;
+ break;
}
mtx_unlock(&sw_dev_mtx);
- return (ENOENT);
+ return (error);
+}
+
+static int
+sysctl_vm_swap_info(SYSCTL_HANDLER_ARGS)
+{
+ struct xswdev xs;
+ int error;
+
+ if (arg2 !=3D 1) /* name length */
+ return (EINVAL);
+ error =3D swap_dev_info(*(int *)arg1, &xs, NULL, 0);
+ if (error !=3D 0)
+ return (error);
+ error =3D SYSCTL_OUT(req, &xs, sizeof(xs));
+ return (error);
}
=20
SYSCTL_INT(_vm, OID_AUTO, nswapdev, CTLFLAG_RD, &nswapdev, 0,
diff --git a/sys/vm/swap_pager.h b/sys/vm/swap_pager.h
index c3366e8..5c716d9 100644
--- a/sys/vm/swap_pager.h
+++ b/sys/vm/swap_pager.h
@@ -75,7 +75,8 @@ struct swdevt {
extern int swap_pager_full;
extern int swap_pager_avail;
=20
-struct swdevt;
+struct xswdev;
+int swap_dev_info(int name, struct xswdev *xs, char *devname, size_t len);
void swap_pager_copy(vm_object_t, vm_object_t, vm_pindex_t, int);
void swap_pager_freespace(vm_object_t, vm_pindex_t, vm_size_t);
void swap_pager_swap_init(void);
--bQyVjumYeq8yaMqA
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (FreeBSD)
iEYEARECAAYFAk41rUgACgkQC3+MBN1Mb4jcCgCbBopXf7BBTgF9YQQNo+1rPhQV
g7EAoJTlQnYCyX0NfmREKoqOj+AxQM2J
=waeg
-----END PGP SIGNATURE-----
--bQyVjumYeq8yaMqA--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110731193017.GR17489>