Date: Wed, 13 Nov 2024 23:21:24 -0300 From: Jose Luis Duran <jlduran@freebsd.org> To: Konstantin Belousov <kib@freebsd.org> Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 62e6ca0f07e4 - main - ps(1): clean up after swapout removal Message-ID: <CAPwQLcdVhmZWKKYAXdut6uD-DUfjwXdMpde=Ob0ET1u6y7taag@mail.gmail.com> In-Reply-To: <202411091725.4A9HPCHC015836@gitrepo.freebsd.org> References: <202411091725.4A9HPCHC015836@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Nov 9, 2024 at 2:25=E2=80=AFPM Konstantin Belousov <kib@freebsd.org= > wrote: > > The branch main has been updated by kib: > > URL: https://cgit.FreeBSD.org/src/commit/?id=3D62e6ca0f07e448da27cb2cc816= 5e749e7fdfcd7e > > commit 62e6ca0f07e448da27cb2cc8165e749e7fdfcd7e > Author: Konstantin Belousov <kib@FreeBSD.org> > AuthorDate: 2024-11-09 01:37:07 +0000 > Commit: Konstantin Belousov <kib@FreeBSD.org> > CommitDate: 2024-11-09 17:22:42 +0000 > > ps(1): clean up after swapout removal > > The process flag P_INMEM is always set. Eliminate all checks for the > bit. Also eliminate LAZY_PS define and code covered by it: we do no= t > have an u-area for long time, and it cannot be swapped out. > > Also eliminate setting controlled by the '-f' switch, but accept it f= or > backward compatibility. > > The 'W' process secondary state (swapped out) is impossible, stop > calculating it. > > Reviewed by: markj > Sponsored by: The FreeBSD Foundation > Differential revision: https://reviews.freebsd.org/D47492 > --- > bin/ps/Makefile | 7 ------- > bin/ps/print.c | 6 +----- > bin/ps/ps.1 | 9 +-------- > bin/ps/ps.c | 41 ++++++++--------------------------------- > 4 files changed, 10 insertions(+), 53 deletions(-) > > diff --git a/bin/ps/Makefile b/bin/ps/Makefile > index a25b6a796ed0..71973b34dd24 100644 > --- a/bin/ps/Makefile > +++ b/bin/ps/Makefile > @@ -2,13 +2,6 @@ PACKAGE=3Druntime > PROG=3D ps > SRCS=3D fmt.c keyword.c nlist.c print.c ps.c > > -# > -# To support "lazy" ps for non root/wheel users > -# add -DLAZY_PS to the cflags. This helps > -# keep ps from being an unnecessary load > -# on large systems. > -# > -CFLAGS+=3D-DLAZY_PS > LIBADD=3D m kvm jail xo > > .include <bsd.prog.mk> > diff --git a/bin/ps/print.c b/bin/ps/print.c > index a3423d8b3956..59631fb66a10 100644 > --- a/bin/ps/print.c > +++ b/bin/ps/print.c > @@ -253,8 +253,6 @@ state(KINFO *k, VARENT *ve __unused) > *cp =3D '?'; > } > cp++; > - if (!(flag & P_INMEM)) > - *cp++ =3D 'W'; > if (k->ki_p->ki_nice < NZERO || k->ki_p->ki_pri.pri_class =3D=3D = PRI_REALTIME) > *cp++ =3D '<'; > else if (k->ki_p->ki_nice > NZERO || k->ki_p->ki_pri.pri_class = =3D=3D PRI_IDLE) > @@ -633,7 +631,7 @@ getpcpu(const KINFO *k) > #define fxtofl(fixpt) ((double)(fixpt) / fscale) > > /* XXX - I don't like this */ > - if (k->ki_p->ki_swtime =3D=3D 0 || (k->ki_p->ki_flag & P_INMEM) = =3D=3D 0) > + if (k->ki_p->ki_swtime =3D=3D 0) > return (0.0); > if (rawcpu) > return (100.0 * fxtofl(k->ki_p->ki_pctcpu)); > @@ -661,8 +659,6 @@ getpmem(KINFO *k) > if (failure) > return (0.0); > > - if ((k->ki_p->ki_flag & P_INMEM) =3D=3D 0) > - return (0.0); > /* XXX want pmap ptpages, segtab, etc. (per architecture) */ > /* XXX don't have info about shared */ > fracmem =3D ((double)k->ki_p->ki_rssize) / mempages; > diff --git a/bin/ps/ps.1 b/bin/ps/ps.1 > index 828239fd2ba9..8ece5b1bbfad 100644 > --- a/bin/ps/ps.1 > +++ b/bin/ps/ps.1 > @@ -159,9 +159,6 @@ does not imply > but works well with it. > .It Fl e > Display the environment as well. > -.It Fl f > -Show command-line and environment information about swapped out processe= s. > -This option is honored only if the UID of the user is 0. > .It Fl G > Display information about processes which are running with the specified > real group IDs. > @@ -358,9 +355,7 @@ the include file > .It Dv "P_TOTAL_STOP" Ta No "0x02000000" Ta "Stopped for system suspend" > .It Dv "P_INEXEC" Ta No "0x04000000" Ta Process is in Xr execve 2 > .It Dv "P_STATCHILD" Ta No "0x08000000" Ta "Child process stopped or exi= ted" > -.It Dv "P_INMEM" Ta No "0x10000000" Ta "Loaded into memory" > -.It Dv "P_SWAPPINGOUT" Ta No "0x20000000" Ta "Process is being swapped o= ut" > -.It Dv "P_SWAPPINGIN" Ta No "0x40000000" Ta "Process is being swapped in= " > +.It Dv "P_INMEM" Ta No "0x10000000" Ta "Always set, unused" > .It Dv "P_PPTRACE" Ta No "0x80000000" Ta "Vforked child issued ptrace(PT= _TRACEME)" > .El > .It Cm flags2 > @@ -491,8 +486,6 @@ The process is a session leader. > The process' parent is suspended during a > .Xr vfork 2 , > waiting for the process to exec or exit. > -.It Li W > -The process is swapped out. > .It Li X > The process is being traced or debugged. > .El > diff --git a/bin/ps/ps.c b/bin/ps/ps.c > index b0af2bdf37ca..49c69bb76084 100644 > --- a/bin/ps/ps.c > +++ b/bin/ps/ps.c > @@ -68,14 +68,6 @@ > #define W_SEP " \t" /* "Whitespace" list separators *= / > #define T_SEP "," /* "Terminate-element" list separ= ators */ > > -#ifdef LAZY_PS > -#define DEF_UREAD 0 > -#define OPT_LAZY_f "f" > -#else > -#define DEF_UREAD 1 /* Always do the more-expensive r= ead. */ > -#define OPT_LAZY_f /* I.e., the `-f' option is not a= dded. */ > -#endif > - > /* > * isdigit takes an `int', but expects values in the range of unsigned c= har. > * This wrapper ensures that values from a 'char' end up in the correct = range. > @@ -92,7 +84,6 @@ int showthreads; /* will threads be shown?= */ > > struct velisthead varlist =3D STAILQ_HEAD_INITIALIZER(varlist); > > -static int forceuread =3D DEF_UREAD; /* Do extra work to get u-area= . */ > static kvm_t *kd; > static int needcomm; /* -o "command" */ > static int needenv; /* -e */ > @@ -154,7 +145,7 @@ static char vfmt[] =3D "pid,state,time,sl,re,pagein,v= sz,rss,lim,tsiz," > "%cpu,%mem,command"; > static char Zfmt[] =3D "label"; > > -#define PS_ARGS "AaCcD:de" OPT_LAZY_f "G:gHhjJ:LlM:mN:O:o:p:rSTt:= U:uvwXxZ" > +#define PS_ARGS "AaCcD:defG:gHhjJ:LlM:mN:O:o:p:rSTt:U:uvwXxZ" > > int > main(int argc, char *argv[]) > @@ -272,12 +263,9 @@ main(int argc, char *argv[]) > case 'e': /* XXX set ufmt */ > needenv =3D 1; > break; > -#ifdef LAZY_PS > case 'f': > - if (getuid() =3D=3D 0 || getgid() =3D=3D 0) > - forceuread =3D 1; > + /* compat */ > break; > -#endif > case 'G': > add_list(&gidlist, optarg); > xkeep_implied =3D 1; > @@ -1276,31 +1264,21 @@ fmt(char **(*fn)(kvm_t *, const struct kinfo_proc= *, int), KINFO *ki, > return (s); > } > > -#define UREADOK(ki) (forceuread || (ki->ki_p->ki_flag & P_INMEM)) > - > static void > saveuser(KINFO *ki) > { > char tdname[COMMLEN + 1]; > char *argsp; > > - if (ki->ki_p->ki_flag & P_INMEM) { > - /* > - * The u-area might be swapped out, and we can't get > - * at it because we have a crashdump and no swap. > - * If it's here fill in these fields, otherwise, just > - * leave them 0. > - */ > - ki->ki_valid =3D 1; > - } else > - ki->ki_valid =3D 0; > + ki->ki_valid =3D 1; > + > /* > * save arguments if needed > */ > if (needcomm) { > if (ki->ki_p->ki_stat =3D=3D SZOMB) { > ki->ki_args =3D strdup("<defunct>"); > - } else if (UREADOK(ki) || (ki->ki_p->ki_args !=3D NULL)) = { Apparently this is missing an explicit check of ki->ki_p->ki_flag, causing the processes in square brackets be printed within parentheses, that is, taking the else path below, and making test: https://ci.freebsd.org/view/Test/job/FreeBSD-main-amd64-test/lastCompletedB= uild/testReport/bin.pkill/pgrep-_s_test/main/ fail. I have already sent a message via Phabricator, but I am unsure if it has visibility there after the commit. I apologize if this is something that's already known. Thank you! > + } else if (ki->ki_p->ki_args !=3D NULL) { > (void)snprintf(tdname, sizeof(tdname), "%s%s", > ki->ki_p->ki_tdname, ki->ki_p->ki_moretdname)= ; > ki->ki_args =3D fmt(kvm_getargv, ki, > @@ -1315,11 +1293,8 @@ saveuser(KINFO *ki) > ki->ki_args =3D NULL; > } > if (needenv) { > - if (UREADOK(ki)) > - ki->ki_env =3D fmt(kvm_getenvv, ki, > - (char *)NULL, (char *)NULL, 0); > - else > - ki->ki_env =3D strdup("()"); > + ki->ki_env =3D fmt(kvm_getenvv, ki, (char *)NULL, > + (char *)NULL, 0); > if (ki->ki_env =3D=3D NULL) > xo_errx(1, "malloc failed"); > } else { > @@ -1479,7 +1454,7 @@ pidmax_init(void) > static void __dead2 > usage(void) > { > -#define SINGLE_OPTS "[-aCcde" OPT_LAZY_f "HhjlmrSTuvwXxZ]" > +#define SINGLE_OPTS "[-aCcdeHhjlmrSTuvwXxZ]" > > xo_error("%s\n%s\n%s\n%s\n%s\n", > "usage: ps [--libxo] " SINGLE_OPTS " [-O fmt | -o fmt]", --=20 Jose Luis Duran
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAPwQLcdVhmZWKKYAXdut6uD-DUfjwXdMpde=Ob0ET1u6y7taag>