Re: [PATCH v1 2/2] drm/xe/xe_ras: Add RAS support for GPU health indicator

[Andi Shyti <andi.shyti@linux.intel.com>]

Hi Rodrigo,

> > 
> > If we need two strings, then we create two files.
> 
> This part I don't agree. This is just the same as the power cases
> which I already had pointed out.
> 
> But right, you do have a point here. Different from power, we don't
> necessarily need to expose all the supported modes + the current one.
> After all, these states won't be that different from state to state.
> 
> We just need to document it properly and then just print:
> either
> 
> ok
> instead of
> [ok] warning critical
> 
> or
> warning
> instead of
> ok [warning] critical
> 
> or critical
> instead of
> ok warning [critical]
> 
> The full string with all the supported modes do sound bloated for
> this case. Considering that the supported cases themselves shouldn't
> change much and that we keep that documented. My bad, I'm sorry for
> the confusion here.

Think of what a miserable life have userspace developers when
parsing power files. They need to parse for the '[', for the ']'
the number of words, etc. We have here an excellent example of
how *not* to do things. :-)

> > If we just need to provide information to system administrators,
> > then we can either use the debugfs or procfs that don't need to
> > be considered as uAPI.
> 
> That is not true. In many data center deployments the 'debugfs'
> is not available and the admin and cluster tools still need this
> information. It needs to be stable API.

Bingo! :-)

Then you need to define an easily parsable, well documented api,
so that it's accessable to everyone, applications and sysadmins.

Overall, I don't think this is a matter of taste, but, anyway,
you are the maintainer and yours is the last word. To me, this
looks incorrect.

Thanks,
Andi