From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752633AbcHJSFz (ORCPT ); Wed, 10 Aug 2016 14:05:55 -0400 Received: from thejh.net ([37.221.195.125]:41711 "EHLO thejh.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752265AbcHJSFu (ORCPT ); Wed, 10 Aug 2016 14:05:50 -0400 Date: Wed, 10 Aug 2016 20:05:46 +0200 From: Jann Horn To: Sonny Rao Cc: Robert Foss , Andrew Morton , Kees Cook , Al Viro , Cyrill Gorcunov , John Stultz , Robin Humble , Mateusz Guzik , Alexey Dobriyan , Janis Danisevskis , calvinowens@fb.com, Michal Hocko , Konstantin Khlebnikov , Vlastimil Babka , Naoya Horiguchi , "Kirill A. Shutemov" , ldufour@linux.vnet.ibm.com, Johannes Weiner , "linux-kernel@vger.kernel.org" , Ben Zhang , Bryan Freed , Filipe Brandenburger Subject: Re: [PACTH v1] mm, proc: Implement /proc//totmaps Message-ID: <20160810180546.GA486@pc.thejh.net> References: <1470758743-17685-1-git-send-email-robert.foss@collabora.com> <20160809192414.GA19573@pc.thejh.net> <8ac1b493-e051-ea0e-3a71-c4476054bdb2@collabora.com> <20160810173719.GA25801@pc.thejh.net> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="5vNYLRcllDrimb99" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --5vNYLRcllDrimb99 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Aug 10, 2016 at 10:45:51AM -0700, Sonny Rao wrote: > On Wed, Aug 10, 2016 at 10:37 AM, Jann Horn wrote: > > On Wed, Aug 10, 2016 at 10:23:53AM -0700, Sonny Rao wrote: > >> On Tue, Aug 9, 2016 at 2:01 PM, Robert Foss wrote: > >> > > >> > > >> > On 2016-08-09 03:24 PM, Jann Horn wrote: > >> >> > >> >> On Tue, Aug 09, 2016 at 12:05:43PM -0400, robert.foss@collabora.com= wrote: > >> >>> > >> >>> From: Sonny Rao > >> >>> > >> >>> This is based on earlier work by Thiago Goncales. It implements a = new > >> >>> per process proc file which summarizes the contents of the smaps f= ile > >> >>> but doesn't display any addresses. It gives more detailed informa= tion > >> >>> than statm like the PSS (proprotional set size). It differs from = the > >> >>> original implementation in that it doesn't use the full blown set = of > >> >>> seq operations, uses a different termination condition, and doesn't > >> >>> displayed "Locked" as that was broken on the original implemenatio= n. > >> >>> > >> >>> This new proc file provides information faster than parsing the > >> >>> potentially > >> >>> huge smaps file. > >> >>> > >> >>> Signed-off-by: Sonny Rao > >> >>> > >> >>> Tested-by: Robert Foss > >> >>> Signed-off-by: Robert Foss > >> >> > >> >> > >> >> > >> >>> +static int totmaps_proc_show(struct seq_file *m, void *data) > >> >>> +{ > >> >>> + struct proc_maps_private *priv =3D m->private; > >> >>> + struct mm_struct *mm; > >> >>> + struct vm_area_struct *vma; > >> >>> + struct mem_size_stats *mss_sum =3D priv->mss; > >> >>> + > >> >>> + /* reference to priv->task already taken */ > >> >>> + /* but need to get the mm here because */ > >> >>> + /* task could be in the process of exiting */ > >> >> > >> >> > >> >> Can you please elaborate on this? My understanding here is that you > >> >> intend for the caller to be able to repeatedly read the same totmaps > >> >> file with pread() and still see updated information after the target > >> >> process has called execve() and be able to detect process death > >> >> (instead of simply seeing stale values). Is that accurate? > >> >> > >> >> I would prefer it if you could grab a reference to the mm_struct > >> >> directly at open time. > >> > > >> > > >> > Sonny, do you know more about the above comment? > >> > >> I think right now the file gets re-opened every time, but the mode > >> where the file is opened once and repeatedly read is interesting > >> because it avoids having to open the file again and again. > >> > >> I guess you could end up with a wierd situation where you don't read > >> the entire contents of the file in open call to read() and you might > >> get inconsistent data across the different statistics? > > > > If the file is read in two chunks, totmaps_proc_show is only called > > once. The patch specifies seq_read as read handler. Have a look at its > > definition. As long as you don't read from the same seq file in > > parallel or seek around in it, simple sequential reads will not > > re-invoke the show() method for data that has already been formatted. > > For partially consumed data, the kernel buffers the rest until someone > > reads it or seeks to another offset. >=20 > Ok that's good. If the consumer were using pread() though, would that > look like a seek? Only if the consumer uses pread() with an offset that is not the same as the end offset of the previous read. So if you tried to use the same file from multiple threads in parallel, you might still have issues, but as long as you don't do that, it should be fine. I guess it might make sense to document this behavior somewhere - maybe the proc.5 manpage? --5vNYLRcllDrimb99 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXq2z6AAoJED4KNFJOeCOoYdIP/jJtQ0IIECxNhz//MyfWAN0I 4D6pbcbG/peAwXhwj/I1M3sC5lUxSLzktI2AvT3KCQOYkJ/8EPys82B8hukyI0QJ VAhhhY4Rv4LjC3YbBBwy0ceeZuQKaKKuxo99KIq7jBHT3WIot2A4oF9O2XxgJxtd SUXMk8V3hcVQONwh8ei+QMZWd56XmpzmOGcEV/6Vy6d2UscekNfcuh6OfopbgTn/ mo8CBIjubDm21vOs8JmA15U4CsuJ9ooBRPfnV99zu4mvg6LLa/30KS+q+2WBVM/e Pnp8NpheW0fDj/77fu7W2mIdnIW26nQEiVcG10dyOpFuPdIpreKNj/bbZN+NHVFe iGDlBvJXsTLxW49B73E6YECs7A/yuLPPEP7TINmKe7YX9qzjc/sP4a2fshUGOks6 GVSTE0MC1Un9irK2XJQ+l9Wt56X7ypho0kz11O8tJZKi4NyYz+5MWcdKp54pc469 40GPkCD6hSnrVRiY3YdqkZSlbtB8+ScRDtu/G3W5gHAqsKckzpPbJVr14WtN9iqy I+wtYwJn+uSF+k/0sZtTjpGsMxTGJryYpljGPcNlIxpWAcgUkXlf9dcgvWi8tA+7 WyKhWL1GszA6h5wSB9OWngCFDXXr86fonqVUTSflN3ZXv6reqYt/YV1+oKThuRZc U2yAZvwq0Pmc+VclGqKc =Pdxy -----END PGP SIGNATURE----- --5vNYLRcllDrimb99--