Date: Sat, 30 Jul 2011 20:32:39 +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> Subject: Re: kern/159281: [PATCH] Linux-like /proc/swaps for linprocfs Message-ID: <20110730173239.GA17489@deviant.kiev.zoral.com.ua> In-Reply-To: <CAOfDtXOJjOoQA8yNFPVdQRqCqr-Vc5nscMbgOLGLMuvTP9mp1w@mail.gmail.com> References: <CAOfDtXOJjOoQA8yNFPVdQRqCqr-Vc5nscMbgOLGLMuvTP9mp1w@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--rwb68Uo94/+7gMfP
Content-Type: text/plain; charset=koi8-r
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Sat, Jul 30, 2011 at 01:06:48AM +0200, Robert Millan wrote:
> Hi Kostik,
>=20
> 2011/7/29 Kostik Belousov <kostikbel@gmail.com>:
> > The patch is too hackish, IMHO.
> > I would prefer to have an exported kernel function that fills xswdev
> > by index, used both by vm_swap_info and linprocfs.
> >
> > For the device name, you would use sw_vp->v_rdev->si_name, see, for
> > instance, the following fragment in the swapoff_all():
> > =9A =9A =9A =9A =9A =9A =9A =9Aif (vn_isdisk(sp->sw_vp, NULL))
> > =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9Adevname =3D sp->sw_vp->v=
_rdev->si_name;
> > =9A =9A =9A =9A =9A =9A =9A =9Aelse
> > =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9A =9Adevname =3D "[file]";
> > This could be another function that returns swap information by index.
>=20
> Here's a patch with the changes you requested.
>=20
There are several issues in the patch that I see, not counting minor
stylistic bugs.
First, the sw_dev_mtx should be kept until all the data is read from
the xs. The locking there is somewhat vague, the original code was probably
correct because sysctl handler is not marked mpsafe, and all functions
changing the swtailq hold Giant.
Second, my proposal contains a flaw. Namely, if some swap device was removed
between calls to swap_info and swap_devname calls, we get mangled list.
The third problem, which is not fixed, and which I do not want to handle,
is that the swap devices list can be changed between calls to swap_devname(=
),
changing device indexes and thus making the output mangled.
Should the swap device name be separated from 'unknown' word by
space or by tab ?
I updated your patch, hopefully fixing the issues. Do you have comments
or objections ?
diff --git a/sys/compat/linprocfs/linprocfs.c b/sys/compat/linprocfs/linpro=
cfs.c
index 692c5a3..0733913 100644
--- a/sys/compat/linprocfs/linprocfs.c
+++ b/sys/compat/linprocfs/linprocfs.c
@@ -502,6 +502,30 @@ linprocfs_dostat(PFS_FILL_ARGS)
return (0);
}
=20
+static int
+linprocfs_doswaps(PFS_FILL_ARGS)
+{
+ struct xswdev xsw;
+ int n;
+ long long total, used;
+ char devname[SPECNAMELEN + 1];
+
+ sbuf_printf(sb, "Filename\t\t\t\tType\t\tSize\tUsed\tPriority\n");
+
+ for (n =3D 0; ; n++) {
+ if (swap_dev_info(n, &xsw, devname, sizeof(devname)) !=3D 0)
+ break;
+
+ total =3D (long long)xsw.xsw_nblks * PAGE_SIZE / 1024;
+ used =3D (long long)xsw.xsw_used * PAGE_SIZE / 1024;
+
+ sbuf_printf(sb, "/dev/%-34s unknown\t\t%lld\t%lld\t-1\n",
+ devname, total, used);
+ }
+
+ return (0);
+}
+
/*
* Filler function for proc/uptime
*/
@@ -1490,6 +1514,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..5929871 100644
--- a/sys/vm/swap_pager.c
+++ b/sys/vm/swap_pager.c
@@ -2365,35 +2365,58 @@ 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;
}
- n++;
+
+ 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);
+ }
+ error =3D 0;
+ break;
}
mtx_unlock(&sw_dev_mtx);
- return (ENOENT);
+ return (error);
+}
+
+static int
+sysctl_vm_swap_info(SYSCTL_HANDLER_ARGS)
+{
+ int *name =3D (int *)arg1;
+ int error;
+ struct xswdev xs;
+
+ if (arg2 !=3D 1) /* name length */
+ return (EINVAL);
+
+ error =3D swap_dev_info(*name, &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..d7b0f5e 100644
--- a/sys/vm/swap_pager.h
+++ b/sys/vm/swap_pager.h
@@ -76,6 +76,8 @@ 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);
--rwb68Uo94/+7gMfP
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (FreeBSD)
iEYEARECAAYFAk40QDcACgkQC3+MBN1Mb4iLaQCfVGDRkZXlUAA4Jq1qbfbpnYtR
cmgAoNiAxP0/hLGsVJomShY6io1TEDYS
=HiWn
-----END PGP SIGNATURE-----
--rwb68Uo94/+7gMfP--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20110730173239.GA17489>