Date: Wed, 9 Nov 2011 14:44:55 +0200 From: Kostik Belousov <kostikbel@gmail.com> To: Mikolaj Golub <trociny@freebsd.org> Cc: freebsd-hackers@freebsd.org, Robert Watson <rwatson@freebsd.org> Subject: Re: "ps -e" without procfs(5) Message-ID: <20111109124455.GW50300@deviant.kiev.zoral.com.ua> In-Reply-To: <86r51iqoad.fsf@kopusha.home.net> References: <20111031094948.GB50300@deviant.kiev.zoral.com.ua> <86vcr21agm.fsf@kopusha.home.net> <20111105135801.GT50300@deviant.kiev.zoral.com.ua> <86ehxmpogp.fsf@kopusha.home.net> <20111105154443.GB50300@deviant.kiev.zoral.com.ua> <86ehxmjsza.fsf@kopusha.home.net> <20111105194553.GK50300@deviant.kiev.zoral.com.ua> <8662iyjof9.fsf@kopusha.home.net> <20111106181041.GH50300@deviant.kiev.zoral.com.ua> <86r51iqoad.fsf@kopusha.home.net>
next in thread | previous in thread | raw e-mail | index | archive | help
--JIU8dMvCuAW53rIa Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Nov 08, 2011 at 11:47:54PM +0200, Mikolaj Golub wrote: >=20 > On Sun, 6 Nov 2011 20:10:41 +0200 Kostik Belousov wrote: >=20 > KB> On Sat, Nov 05, 2011 at 10:37:46PM +0200, Mikolaj Golub wrote: > >>=20 > >> http://people.freebsd.org/~trociny/env.sys.3.patch >=20 > KB> Oops, I missed this in the previous review. You cannot use fubyte in > KB> proc_read_mem(). fubyte reads a byte from the address space of the c= urrent > KB> process. The fix is easy, use proc_rwmem for 1 byte. >=20 > KB> I do not think that fall back to single byte read is warranted for > KB> proc_read_mem calls e.g. for ps_strings. Add a flag to indicate whet= her > KB> the proc_read_mem should fall back to byte read ? >=20 > KB> I would prefer using sizeof(uint64_t) and sizeof(uint32_t) instead o= f 8 > KB> and 4 constants in the align checks. >=20 > KB> Might be, add PROC_ASSERT_HELD() to get_ps_string() ? >=20 > KB> procfs patch looks good. >=20 > Thanks. The updated version: >=20 > http://people.freebsd.org/~trociny/env.sys.4.patch >=20 > Investigating cases when EFAULT was returned and if the fallback was > successful I noticed that most of the cases were when p->p_comm changed d= uring > the read, so the process was in exec in that time. In order to avoid this > error I added a check for P_INEXEC flag. And now you return success and nothing gets copied out for the process in P_INEXEC state. Either you should return an error like EAGAIN, or consider the P_INEXEC state as transitional and wait till process leaves it. Or, ignore the state as it was before, and return whatever error proc_rwmem generated (my preference). >=20 > After this I observed EFAULT (very rarely) only when reading arg or env > strings and fallback was successful for those cases. So I modified the pa= tch > to do fallback only when reading strings (as it was in one of my earlier > versions but with wrong fubyte), and returned your comment which explains= why > it may happen :-) >=20 > Also in the procfs patch I have added the check for process state. >=20 > The userland part has not been changed since my first report: >=20 > http://people.freebsd.org/~trociny/env.user.patch >=20 > --=20 > Mikolaj Golub --JIU8dMvCuAW53rIa Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.18 (FreeBSD) iEYEARECAAYFAk66dccACgkQC3+MBN1Mb4iarACgvhzunOIjpRYaGRExe96olR3j BQgAnRIhrLZcozVy/okSIulpv+zrmM4+ =w/U9 -----END PGP SIGNATURE----- --JIU8dMvCuAW53rIa--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20111109124455.GW50300>