From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752768AbcHLQ21 (ORCPT ); Fri, 12 Aug 2016 12:28:27 -0400 Received: from bhuna.collabora.co.uk ([46.235.227.227]:46496 "EHLO bhuna.collabora.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752372AbcHLQ2Z (ORCPT ); Fri, 12 Aug 2016 12:28:25 -0400 Subject: Re: [PACTH v1] mm, proc: Implement /proc//totmaps To: Jann Horn , Sonny Rao 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> <20160810180546.GA486@pc.thejh.net> Cc: 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 From: Robert Foss Message-ID: Date: Fri, 12 Aug 2016 12:28:11 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <20160810180546.GA486@pc.thejh.net> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016-08-10 02:05 PM, Jann Horn wrote: > 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 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 = m->private; >>>>>>> + struct mm_struct *mm; >>>>>>> + struct vm_area_struct *vma; >>>>>>> + struct mem_size_stats *mss_sum = 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. >> >> 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? > I'll add a note about limitations for parallel read. The overall documentation for this feature should live in the proc.5 manpage as well?