Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 12 Feb 2020 12:42:43 -0800
From:   Roman Gushchin <guro@fb.com>
To:     Bharata B Rao <bharata@linux.ibm.com>
CC:     "linux-mm@kvack.org" <linux-mm@kvack.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Michal Hocko <mhocko@kernel.org>,
        Johannes Weiner <hannes@cmpxchg.org>,
        Shakeel Butt <shakeelb@google.com>,
        Vladimir Davydov <vdavydov.dev@gmail.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Kernel Team <Kernel-team@fb.com>,
        Yafang Shao <laoar.shao@gmail.com>
Subject: Re: [PATCH v2 27/28] tools/cgroup: make slabinfo.py compatible with
 new slab controller
Message-ID: <20200212204243.GA177958@carbon.lan>
References: <20200127173453.2089565-1-guro@fb.com>
 <20200127173453.2089565-28-guro@fb.com>
 <20200130021729.GB21973@in.ibm.com>
 <20200131222346.GA20909@xps>
 <20200212052124.GA12769@in.ibm.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200212052124.GA12769@in.ibm.com>
MIME-Version: 1.0
Received-SPF: None (protection.outlook.com: fb.com does not designate
 permitted sender hosts)
X-MS-Exchange-CrossTenant-Network-Message-Id: 69589680-f2c5-486d-731b-08d7affc1fba
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Feb 2020 20:42:49.7025
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: zVmuC5UOPEFnPUT0wc8BSMRTbGLZD9sL0MGmKF1wXgtbeUbsu9t2V3UUNgjHYGOD
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR15MB2551
X-OriginatorOrg: fb.com
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.138,18.0.572
 definitions=2020-02-12_09:2020-02-12,2020-02-12 signatures=0
X-Proofpoint-Spam-Details: rule=fb_default_notspam policy=fb_default score=0 adultscore=0
 suspectscore=2 lowpriorityscore=0 bulkscore=0 priorityscore=1501
 spamscore=0 impostorscore=0 clxscore=1015 phishscore=0 mlxlogscore=999
 malwarescore=0 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2001150001 definitions=main-2002120142
X-FB-Internal: deliver
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Feb 12, 2020 at 10:51:24AM +0530, Bharata B Rao wrote:
> On Fri, Jan 31, 2020 at 10:24:58PM +0000, Roman Gushchin wrote:
> > On Thu, Jan 30, 2020 at 07:47:29AM +0530, Bharata B Rao wrote:
> > > On Mon, Jan 27, 2020 at 09:34:52AM -0800, Roman Gushchin wrote:
> > 
> > Btw, I've checked that the change like you've done above fixes the problem.
> > The script works for me both on current upstream and new_slab.2 branch.
> > 
> > Are you sure that in your case there is some kernel memory charged to that
> > cgroup? Please note, that in the current implementation kmem_caches are created
> > on demand, so the accounting is effectively enabled with some delay.
> 
> I do see kmem getting charged.
> 
> # cat /sys/fs/cgroup/memory/1/memory.kmem.usage_in_bytes /sys/fs/cgroup/memory/1/memory.usage_in_bytes
> 182910976
> 4515627008

Great.

> 
> > Below is an updated version of the patch to use:
> 
> I see the below failure with this updated version:

Are you sure that drgn is picking right symbols?
I had a similar transient issue during my work, when drgn was actually
using symbols from a different kernel.

> 
> # ./tools/cgroup/slabinfo-new.py /sys/fs/cgroup/memory/1
> # name            <active_objs> <num_objs> <objsize> <objperslab> <pagesperslab> : tunables <limit> <batchcount> <sharedfactor> : slabdata <active_slabs> <num_slabs> <sharedavail>
> Traceback (most recent call last):
>   File "/usr/local/bin/drgn", line 11, in <module>
>     sys.exit(main())
>   File "/usr/local/lib/python3.6/dist-packages/drgn/internal/cli.py", line 127, in main
>     runpy.run_path(args.script[0], init_globals=init_globals, run_name="__main__")
>   File "/usr/lib/python3.6/runpy.py", line 263, in run_path
>     pkg_name=pkg_name, script_name=fname)
>   File "/usr/lib/python3.6/runpy.py", line 96, in _run_module_code
>     mod_name, mod_spec, pkg_name, script_name)
>   File "/usr/lib/python3.6/runpy.py", line 85, in _run_code
>     exec(code, run_globals)
>   File "./tools/cgroup/slabinfo-new.py", line 158, in <module>
>     main()
>   File "./tools/cgroup/slabinfo-new.py", line 153, in main
>     memcg.kmem_caches.address_of_(),
> AttributeError: 'struct mem_cgroup' has no member 'kmem_caches'
> 
> > +
> > +def main():
> > +    parser = argparse.ArgumentParser(description=DES,C
> > +                                     formatter_class=
> > +                                     argparse.RawTextHelpFormatter)
> > +    parser.add_argument('cgroup', metavar='CGROUP',
> > +                        help='Target memory cgroup')
> > +    args = parser.parse_args()
> > +
> > +    try:
> > +        cgroup_id = stat(args.cgroup).st_ino
> > +        find_memcg_ids()
> > +        memcg = MEMCGS[cgroup_id]
> > +    except KeyError:
> > +        err('Can\'t find the memory cgroup')
> > +
> > +    cfg = detect_kernel_config()
> > +
> > +    print('# name            <active_objs> <num_objs> <objsize> <objperslab> <pagesperslab>'
> > +          ' : tunables <limit> <batchcount> <sharedfactor>'
> > +          ' : slabdata <active_slabs> <num_slabs> <sharedavail>')
> > +
> > +    for s in list_for_each_entry('struct kmem_cache',
> > +                                 memcg.kmem_caches.address_of_(),
> > +                                 'memcg_params.kmem_caches_node'):
> 
> Are you sure this is the right version? In the previous version
> you had the if-else loop that handled shared_slab_pages and old
> scheme separately.

Which one you're refering to?

As in my tree there are two patches:
fa490da39afb tools/cgroup: add slabinfo.py tool
e3bee81aab44 tools/cgroup: make slabinfo.py compatible with new slab controller

The second one adds the if clause you're probably referring to.

Thanks!

Roman