From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933477AbcHJSOG (ORCPT ); Wed, 10 Aug 2016 14:14:06 -0400 Received: from thejh.net ([37.221.195.125]:41756 "EHLO thejh.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933033AbcHJSOC (ORCPT ); Wed, 10 Aug 2016 14:14:02 -0400 Date: Wed, 10 Aug 2016 19:37:19 +0200 From: Jann Horn To: Sonny Rao Cc: Robert Foss , Andrew Morton , Kees Cook , viro@zeniv.linux.org.uk, gorcunov@openvz.org, John Stultz , plaguedbypenguins@gmail.com, Mateusz Guzik , adobriyan@gmail.com, jdanis@google.com, calvinowens@fb.com, mhocko@suse.com, koct9i@gmail.com, vbabka@suse.cz, n-horiguchi@ah.jp.nec.com, kirill.shutemov@linux.intel.com, 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: <20160810173719.GA25801@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> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="EeQfGwPcQSOJBaQU" 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 --EeQfGwPcQSOJBaQU Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Aug 10, 2016 at 10:23:53AM -0700, Sonny Rao wrote: > On Tue, Aug 9, 2016 at 2:01 PM, Robert Foss w= rote: > > > > > > On 2016-08-09 03:24 PM, Jann Horn wrote: > >> > >> On Tue, Aug 09, 2016 at 12:05:43PM -0400, robert.foss@collabora.com wr= ote: > >>> > >>> 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 file > >>> but doesn't display any addresses. It gives more detailed information > >>> 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 implemenation. > >>> > >>> 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? >=20 > 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. >=20 > 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. --EeQfGwPcQSOJBaQU Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXq2ZPAAoJED4KNFJOeCOopX4QAL0iSvau2T27QicUzpeWEhPG jvskCL2OKypUXs+HXrhmxkDurx03007sqjoiN0R5D1hdjGnJyAiRruV/esuM0O4U IKmm335ifssYYDlxB2tH9C3KYjpj1gmV3WGspRh0NEsoLJ+E6TmGzopeg8LMQlQN /+6KZcuDtIn+YoVWR6BTqR/kiVC82D2H8liOJGJheCjf/hfVsDQlKIdAmGOMSnvb FK47iZW4GLHmC/avihVHVaYZ33D5DibIGytXWRIAm38hOFYW0gH+1XnxAosDw2se q+Ap4k/IfO17K1sTSWfYJl25YKhCQO/xPmsItMkREGYxt3lO4nYU9eHCpVWRkdcW haF1Sx8HstDtDmgLXR2fR7gVLMaewOqn1RzLGXs1FCdT1rcoaYALganjxusCTGWa rQXYr+pSZcbW8jgAZ9wwVmr8oerYYUTmNmcfUZ7HWYaBD6342TG+1xxFL5Mwv6rC 7kI0J01E+jxgTW7UATemViHkH1w1YkAj5y8l6nv9niNoUbCTTGPSJMoTQtqniKh5 vE4h9pWTxLq27Kc9MWbIBcH+/vIqKkWR5FNYwqEZo4PrXLJ5dSP+o+ThZqsG8ed1 tWUiTSemwZeX5jwMlIs360W/yLs+byRA9VNq6zbh5SqeAnJUneHGVqEqlQpc4wyD 6/xVAEA5UZKr9GO0DEtY =QPen -----END PGP SIGNATURE----- --EeQfGwPcQSOJBaQU--