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

From: mark gross
Date: Sun Sep 03 2017 - 19:41:53 EST


On Wed, Aug 30, 2017 at 07:30:58PM -0400, Steven Rostedt wrote:
> 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?

perhaps, but what I was trying to point out was: "multi line debugFS printf's
like this are very easy to change to append other information and you might
want to worry about the ABI implications that could happen in the future."

--mark
>
> > 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
> --
> To unsubscribe from this list: send the line "unsubscribe linux-edac" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html