[RFC] Modern device mapper module makes problems for dump analysis

[RFC] Modern device mapper module makes problems for dump analysis

[Alexey Ishchuk]

Hi,

in the modern device mapper Linux kernel module the data structure 
struct dm_table is declared more than once. One of those declarations is 
the real structure definition and the other are dummy definitions. This 
coding manner makes serious problems for the Linux kernel dump analysis 
with crash utility using custom EPPIC language scripts and even the 
dminfo built-in crash extension does not work with the dumps. The 
problem occurs because the crash utility tries to expose to the EPPIC 
language scripts a dummy structure definition that contains no required 
fields.

I would like to get to know, why do we need more than one struct 
dm_table declarations in the kernel module? Is it possible to improve 
the device mapper kernel module code to have the only one struct 
dm_table declaration to allow kernel dumps to be analyzed using custom 
scripts?

Best regards,
Alexey Ishchuk

Re: Modern device mapper module makes problems for dump analysis

[Mike Snitzer]

On Tue, Dec 01 2015 at  8:34am -0500,
Alexey Ishchuk <aishchuk@linux.vnet.ibm.com> wrote:

> Hi,
> 
> in the modern device mapper Linux kernel module the data structure
> struct dm_table is declared more than once. One of those
> declarations is the real structure definition and the other are
> dummy definitions. This coding manner makes serious problems for the
> Linux kernel dump analysis with crash utility using custom EPPIC
> language scripts and even the dminfo built-in crash extension does
> not work with the dumps. The problem occurs because the crash
> utility tries to expose to the EPPIC language scripts a dummy
> structure definition that contains no required fields.
> 
> I would like to get to know, why do we need more than one struct
> dm_table declarations in the kernel module? Is it possible to
> improve the device mapper kernel module code to have the only one
> struct dm_table declaration to allow kernel dumps to be analyzed
> using custom scripts?

The dm.c definition is:

/*
 * A dummy definition to make RCU happy.
 * struct dm_table should never be dereferenced in this file.
 */
struct dm_table {
        int undefined__;
};

As you can see in the block comment above this dummy definition is
purely to "make RCU happy"...

We'll need to research how/if we can avoid such hacks (and still have
RCU function as needed).

But short of eliminating the dummy definition, have you tried using the
crash utility's 'set scope <text_address>' capability to force the use
of the dm-table.c definition? e.g.:

crash> mod -s dm_mod
     MODULE       NAME                     SIZE  OBJECT FILE
ffffffffa0013640  dm_mod                 110592  /lib/modules/4.4.0-rc1.snitm+/kernel/drivers/md/dm-mod.ko

crash> struct dm_table
struct dm_table {
    int undefined__;
}
SIZE: 4

crash> set scope dm_table_create
scope: ffffffffa0005b30 (dm_table_create)

crash> struct dm_table
struct dm_table {
    struct mapped_device *md;
    unsigned int type;
    unsigned int depth;
    unsigned int counts[16];
    sector_t *index[16];
    unsigned int num_targets;
    unsigned int num_allocated;
    sector_t *highs;
    struct dm_target *targets;
    struct target_type *immutable_target_type;
    unsigned int integrity_supported : 1;
    unsigned int singleton : 1;
    fmode_t mode;
    struct list_head devices;
    void (*event_fn)(void *);
    void *event_context;
    struct dm_md_mempools *mempools;
    struct list_head target_callbacks;
}
SIZE: 304

Re: Modern device mapper module makes problems for dump analysis

[Mikulas Patocka]

On Thu, 3 Dec 2015, Mike Snitzer wrote:

> On Tue, Dec 01 2015 at  8:34am -0500,
> Alexey Ishchuk <aishchuk@linux.vnet.ibm.com> wrote:
> 
> > Hi,
> > 
> > in the modern device mapper Linux kernel module the data structure
> > struct dm_table is declared more than once. One of those
> > declarations is the real structure definition and the other are
> > dummy definitions. This coding manner makes serious problems for the
> > Linux kernel dump analysis with crash utility using custom EPPIC
> > language scripts and even the dminfo built-in crash extension does
> > not work with the dumps. The problem occurs because the crash
> > utility tries to expose to the EPPIC language scripts a dummy
> > structure definition that contains no required fields.
> > 
> > I would like to get to know, why do we need more than one struct
> > dm_table declarations in the kernel module? Is it possible to
> > improve the device mapper kernel module code to have the only one
> > struct dm_table declaration to allow kernel dumps to be analyzed
> > using custom scripts?
> 
> The dm.c definition is:
> 
> /*
>  * A dummy definition to make RCU happy.
>  * struct dm_table should never be dereferenced in this file.
>  */
> struct dm_table {
>         int undefined__;
> };
> 
> As you can see in the block comment above this dummy definition is
> purely to "make RCU happy"...
> 
> We'll need to research how/if we can avoid such hacks (and still have
> RCU function as needed).

The function __rcu_dereference_check does:
((typeof(*p) __force __kernel *)(________p1));

--- and that typeof(*p) will not compile if the structure pointerd to by p 
isn't defined. You could change "typeof(*p) *" to "typeof(p)" and it will 
compile, but I don't know what to do with the __force and __kernel flags.

Mikulas


> But short of eliminating the dummy definition, have you tried using the
> crash utility's 'set scope <text_address>' capability to force the use
> of the dm-table.c definition? e.g.:
> 
> crash> mod -s dm_mod
>      MODULE       NAME                     SIZE  OBJECT FILE
> ffffffffa0013640  dm_mod                 110592  /lib/modules/4.4.0-rc1.snitm+/kernel/drivers/md/dm-mod.ko
> 
> crash> struct dm_table
> struct dm_table {
>     int undefined__;
> }
> SIZE: 4
> 
> crash> set scope dm_table_create
> scope: ffffffffa0005b30 (dm_table_create)
> 
> crash> struct dm_table
> struct dm_table {
>     struct mapped_device *md;
>     unsigned int type;
>     unsigned int depth;
>     unsigned int counts[16];
>     sector_t *index[16];
>     unsigned int num_targets;
>     unsigned int num_allocated;
>     sector_t *highs;
>     struct dm_target *targets;
>     struct target_type *immutable_target_type;
>     unsigned int integrity_supported : 1;
>     unsigned int singleton : 1;
>     fmode_t mode;
>     struct list_head devices;
>     void (*event_fn)(void *);
>     void *event_context;
>     struct dm_md_mempools *mempools;
>     struct list_head target_callbacks;
> }
> SIZE: 304
>