Re: [PATCH 0/7] EDAC, mce_amd: Issue decoded MCE through the tracepoint

[Steven Rostedt]

On Wed, 30 Aug 2017 14:47:19 -0700
mark gross <mgross@xxxxxxxxxxxxxxx> wrote:


> >  struct dentry *ras_debugfs_dir;
> >  
> >  static atomic_t trace_count = ATOMIC_INIT(0);
> > @@ -12,7 +14,9 @@ EXPORT_SYMBOL_GPL(ras_userspace_consumers);
> >  
> >  static int trace_show(struct seq_file *m, void *v)
> >  {
> > -	return atomic_read(&trace_count);
> > +	seq_printf(m, "readers:%d\n", atomic_read(&trace_count));
> > +	seq_printf(m, "has decoder:%d\n", mca_cfg.has_decoder);  
> 
> do you want to worry about this debugfs interfaces as ABI?

It's the old, if a tree falls in the forest issue. If you break the ABI
but nobody is around to notice, did it really break?

> debugfs changes have bitten me on one specific OS in irritating ways.
> 
> I'm not sure what the word is for debugfs interfaces as ABI these days so this
> feedback may be not so useful.

I discussed this with Boris before he sent it out. The current code is
actually broken. The trace_show() looks to be just a stub for a hack to
have userspace tell the kernel it's reading something (the
trace_count). The trace_show() function here returns the trace_count,
which is just wrong. The seq_file show function is suppose to return
less than zero on error, zero on success, or greater than zero if the
show is suppose to skip the current field but not error out. This
trace_show() function doesn't actually show anything. If you cat the
file, it will be blank. Returning zero or greater than zero has the
same effect. Which is nothing.

-- Steve