Re: [PATCH] dma-debug: add dumping facility via debugfs

[LABBE Corentin]

On Wed, Jan 16, 2019 at 06:10:13PM +0000, Robin Murphy wrote:
> On 16/01/2019 13:44, Corentin Labbe wrote:
> > While debugging a DMA mapping leak, I needed to access
> > debug_dma_dump_mappings() but easily from user space.
> > 
> > This patch adds a /sys/kernel/debug/dma-api/dump file which contain all
> > current DMA mapping.
> > 
> > Signed-off-by: Corentin Labbe <clabbe@xxxxxxxxxxxx>
> > ---
> >   kernel/dma/debug.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 47 insertions(+)
> > 
> > diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
> > index 23cf5361bcf1..9253382f5729 100644
> > --- a/kernel/dma/debug.c
> > +++ b/kernel/dma/debug.c
> > @@ -144,6 +144,7 @@ static struct dentry *num_free_entries_dent __read_mostly;
> >   static struct dentry *min_free_entries_dent __read_mostly;
> >   static struct dentry *nr_total_entries_dent __read_mostly;
> >   static struct dentry *filter_dent           __read_mostly;
> > +static struct dentry *dump_dent             __read_mostly;
> >   
> >   /* per-driver filter related state */
> >   
> > @@ -840,6 +841,47 @@ static const struct file_operations filter_fops = {
> >   	.llseek = default_llseek,
> >   };
> >   
> > +static int dump_read(struct seq_file *seq, void *v)
> > +{
> > +	int idx;
> > +
> > +	for (idx = 0; idx < HASH_SIZE; idx++) {
> > +		struct hash_bucket *bucket = &dma_entry_hash[idx];
> > +		struct dma_debug_entry *entry;
> > +		unsigned long flags;
> > +
> > +		spin_lock_irqsave(&bucket->lock, flags);
> > +
> > +		list_for_each_entry(entry, &bucket->list, list) {
> > +			seq_printf(seq,
> > +				   "%s %s %s idx %d P=%llx N=%lx D=%llx L=%llx %s %s\n",
> > +				   dev_name(entry->dev),
> > +				   dev_driver_string(entry->dev),
> > +				   type2name[entry->type], idx,
> > +				   phys_addr(entry), entry->pfn,
> > +				   entry->dev_addr, entry->size,
> > +				   dir2name[entry->direction],
> > +				   maperr2str[entry->map_err_type]);
> > +		}
> > +
> > +		spin_unlock_irqrestore(&bucket->lock, flags);
> > +	}
> > +	return 0;
> > +}
> 
> It's a shame that this is pretty much a duplication of 
> debug_dma_dump_mappings(), but there seems no straightforward way to 
> define one in terms of the other :/
> 
> (although given that we'd rather not have that weird external interface 
> anyway, maybe "now you can use debugfs instead" might be justification 
> enough for cleaning it up...)
> 
> > +
> > +static int dump_open(struct inode *inode, struct file *file)
> > +{
> > +	return single_open(file, dump_read, inode->i_private);
> > +}
> > +
> > +static const struct file_operations dump_fops = {
> > +	.owner = THIS_MODULE,
> > +	.open = dump_open,
> > +	.read = seq_read,
> > +	.llseek = seq_lseek,
> > +	.release = single_release,
> > +};
> > +
> 
> DEFINE_SHOW_ATTRIBUTE()?
> 

I will use it

Thanks