From owner-freebsd-hackers@FreeBSD.ORG Sun Jul 31 19:30:26 2011 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id AC75E1065670; Sun, 31 Jul 2011 19:30:26 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.zoral.com.ua (mx0.zoral.com.ua [91.193.166.200]) by mx1.freebsd.org (Postfix) with ESMTP id EE2C58FC12; Sun, 31 Jul 2011 19:30:25 +0000 (UTC) Received: from deviant.kiev.zoral.com.ua (root@deviant.kiev.zoral.com.ua [10.1.1.148]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id p6VJUHZU064743 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 31 Jul 2011 22:30:17 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.4/8.14.4) with ESMTP id p6VJUH04022401; Sun, 31 Jul 2011 22:30:17 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.4/8.14.4/Submit) id p6VJUH1R022400; Sun, 31 Jul 2011 22:30:17 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Sun, 31 Jul 2011 22:30:17 +0300 From: Kostik Belousov To: Robert Millan Message-ID: <20110731193017.GR17489@deviant.kiev.zoral.com.ua> References: <20110730173239.GA17489@deviant.kiev.zoral.com.ua> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="bQyVjumYeq8yaMqA" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: clamav-milter 0.95.2 at skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-3.3 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00, DNS_FROM_OPENWHOIS autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua Cc: freebsd-hackers@freebsd.org, freebsd-emulation@freebsd.org, Ed Maste , bde@freebsd.org Subject: Re: kern/159281: [PATCH] Linux-like /proc/swaps for linprocfs X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 31 Jul 2011 19:30:26 -0000 --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 : > > 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--