Message-ID: <56A39BD9.8020204@siemens.com>
Date:	Sat, 23 Jan 2016 16:27:21 +0100
From:	Jan Kiszka <jan.kiszka@...mens.com>
To:	Kieran Bingham <kieran.bingham@...aro.org>
Cc:	linux-kernel@...r.kernel.org, maxime.coquelin@...com,
	peter.griffin@...aro.org, lee.jones@...aro.org
Subject: Re: [PATCH 4/5] scripts/gdb: Add mount point list command

On 2016-01-20 12:15, Kieran Bingham wrote:
> lx-mounts will identify current mount points based on the 'init_task'
> namespace by default, as we do not yet have a kernel thread list
> implementation to select the current running thread.
> 
> Optionally, a user can specify a PID to list from that process'
> namespace
> 
> This is somewhat limited vs the /proc/mounts file, as that calls into
> vfs hooks through the s_op functions to obtain extra information.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@...aro.org>
> ---
> 
> 
> In this patch, I'm interested in your opinions on coding styles.
> Would you prefer to see the function helpers, (dentry_name, info_opts) where
> they are, or inside the command as class members? Or perhaps defined in utils?

Do you think they could be useful beyond this class? If not, stick them
inside. Refactoring can still be done once they are needed.

> 
> This also shows where I need to take constant information from the kernel.
> In this case, they are simple numerical bitflags, and unlikely to change but
> I didn't want to duplicate their values.
> 
> 
>  scripts/gdb/linux/constants.py.in |  21 ++++++++
>  scripts/gdb/linux/proc.py         | 110 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 131 insertions(+)
> 
> diff --git a/scripts/gdb/linux/constants.py.in b/scripts/gdb/linux/constants.py.in
> index d84084ac945b..739a15d2e984 100644
> --- a/scripts/gdb/linux/constants.py.in
> +++ b/scripts/gdb/linux/constants.py.in
> @@ -12,7 +12,11 @@
>   *
>   */
>  
> +#include <linux/fs.h>
> +#include <linux/mount.h>
> +
>  /* We need to stringify expanded macros so that they can be parsed */
>  #define STRING(x) #x
>  #define XSTRING(x) STRING(x)
>  
> @@ -20,3 +24,20 @@
>  <!-- end-c-headers -->
>  
>  import gdb
> +
> +/* linux/fs.h */
> +LX_MS_RDONLY = MS_RDONLY
> +LX_MS_SYNCHRONOUS = MS_SYNCHRONOUS
> +LX_MS_MANDLOCK = MS_MANDLOCK
> +LX_MS_DIRSYNC = MS_DIRSYNC
> +LX_MS_NOATIME = MS_NOATIME
> +LX_MS_NODIRATIME = MS_NODIRATIME
> +
> +/* linux/mount.h */
> +LX_MNT_NOSUID = MNT_NOSUID
> +LX_MNT_NODEV = MNT_NODEV
> +LX_MNT_NOEXEC = MNT_NOEXEC
> +LX_MNT_NOATIME = MNT_NOATIME
> +LX_MNT_NODIRATIME = MNT_NODIRATIME
> +LX_MNT_RELATIME = MNT_RELATIME
> +
> diff --git a/scripts/gdb/linux/proc.py b/scripts/gdb/linux/proc.py
> index d855b2fd9a06..b79ce2a33a3d 100644
> --- a/scripts/gdb/linux/proc.py
> +++ b/scripts/gdb/linux/proc.py
> @@ -12,6 +12,10 @@
>  #
>  
>  import gdb
> +from linux import constants
> +from linux import utils
> +from linux import tasks
> +from linux import lists
>  
>  
>  class LxCmdLine(gdb.Command):
> @@ -96,3 +100,109 @@ Equivalent to cat /proc/ioports on a running target"""
>          return show_lx_resources("ioport_resource")
>  
>  LxIOPorts()
> +
> +
> +# Mount namespace viewer
> +#  /proc/mounts
> +
> +
> +def dentry_name(d):
> +    if d['d_parent'] == d:
> +        return ""
> +    p = dentry_name(d['d_parent']) + "/"
> +    return p + d['d_iname'].string()
> +
> +
> +def info_opts(lst, opt):
> +    opts = ""
> +    for key, string in lst.items():
> +        if opt & key:
> +            opts += string
> +    return opts
> +
> +
> +FS_INFO = {constants.LX_MS_SYNCHRONOUS: ",sync",
> +           constants.LX_MS_MANDLOCK: ",mand",
> +           constants.LX_MS_DIRSYNC: ",dirsync",
> +           constants.LX_MS_NOATIME: ",noatime",
> +           constants.LX_MS_NODIRATIME: ",nodiratime"}
> +
> +MNT_INFO = {constants.LX_MNT_NOSUID: ",nosuid",
> +            constants.LX_MNT_NODEV: ",nodev",
> +            constants.LX_MNT_NOEXEC: ",noexec",
> +            constants.LX_MNT_NOATIME: ",noatime",
> +            constants.LX_MNT_NODIRATIME: ",nodiratime",
> +            constants.LX_MNT_RELATIME: ",relatime"}
> +
> +mount_type = utils.CachedType("struct mount")
> +mount_ptr_type = mount_type.get_type().pointer()
> +
> +
> +class LxMounts(gdb.Command):
> +    """Report the VFS mounts of the current process namespace.
> +
> +Equivalent to cat /proc/mounts on a running target
> +An integer value can be supplied to display the mount
> +values of that process namespace"""
> +
> +    def __init__(self):
> +        super(LxMounts, self).__init__("lx-mounts", gdb.COMMAND_DATA)
> +
> +    # Equivalent to proc_namespace.c:show_vfsmnt
> +    # However, that has the ability to call into s_op functions
> +    # whereas we cannot and must make do with the information we can obtain.
> +    def invoke(self, arg, from_tty):
> +        argv = gdb.string_to_argv(arg)
> +        if len(argv) >= 1:
> +            try:
> +                pid = int(argv[0])
> +            except:
> +                raise gdb.GdbError("Provide a PID as integer value")
> +        else:
> +            pid = 1
> +
> +        task = tasks.get_task_by_pid(pid)
> +        if not task:
> +            raise gdb.GdbError("Couldn't find a process with PID {}"
> +                               .format(pid))
> +
> +        namespace = task['nsproxy']['mnt_ns']
> +        if not namespace:
> +            raise gdb.GdbError("No namespace for current process")
> +
> +        for vfs in lists.items(mount_ptr_type, "mnt_list", namespace['list']):
> +            # There appears to be a null entry at the end of the list...

"There appears to be" - hmm... Did you check this against the code?

> +            if not vfs['mnt_parent']:
> +                break
> +
> +            devname = vfs['mnt_devname'].string()
> +            devname = devname if devname else "none"
> +
> +            pathname = ""
> +            parent = vfs
> +            while True:
> +                mntpoint = parent['mnt_mountpoint']
> +                pathname = dentry_name(mntpoint) + pathname

I'm getting an error in this line:

(gdb) lx-mounts
devtmpfs /dev devtmpfs rw,relatime 0 0
tmpfs /dev/shm tmpfs rw,relatime 0 0
devpts /dev/pts devpts rw,relatime 0 0
/dev/sda2 / ext4 rw,relatime 0 0
proc /proc proc rw,nodiratime,relatime 0 0
sysfs /sys sysfs rw,relatime 0 0
debugfs /sys/kernel/debug debugfs rw,relatime 0 0
securityfs /sys/kernel/security securityfs rw,relatime 0 0
fusectl /sys/fs/fuse/connections fusectl rw,relatime 0 0
gvfs-fuse-daemon /home/jan/.gvfs fuse rw,relatime,nosuid,nodev 0 0
tracefs /sys/kernel/debug/tracing tracefs rw,relatime 0 0
Traceback (most recent call last):
  File "/data/linux/build-dbg/scripts/gdb/linux/proc.py", line 185, in invoke
    pathname = dentry_name(mntpoint) + pathname
  File "/data/linux/build-dbg/scripts/gdb/linux/proc.py", line 112, in dentry_name
    p = dentry_name(d['d_parent']) + "/"
  File "/data/linux/build-dbg/scripts/gdb/linux/proc.py", line 110, in dentry_name
    if d['d_parent'] == d:
gdb.MemoryError: Cannot access memory at address 0x40
Error occurred in Python command: Cannot access memory at address 0x40

In this case, the dump should have stopped after the tracefs line.

> +                if (parent == parent['mnt_parent']):
> +                    break
> +                parent = parent['mnt_parent']
> +
> +            if (pathname == ""):
> +                pathname = "/"
> +
> +            superblock = vfs['mnt']['mnt_sb']
> +            fstype = superblock['s_type']['name'].string()
> +            s_flags = int(superblock['s_flags'])
> +            m_flags = int(vfs['mnt']['mnt_flags'])
> +            rd = "ro" if (s_flags & constants.LX_MS_RDONLY) else "rw"
> +
> +            gdb.write(
> +                "{} {} {} {}{}{} 0 0\n"
> +                .format(devname,
> +                        pathname,
> +                        fstype,
> +                        rd,
> +                        info_opts(FS_INFO, s_flags),
> +                        info_opts(MNT_INFO, m_flags)))
> +
> +LxMounts()
> 

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

Powered by blists - more mailing lists