[MODERATED] Re: [PATCH 4/4] walnut 4

[Greg KH <gregkh@linuxfoundation.org>]

I was working on the backport to 4.9.y and ran across merge issues with
this patch.  I understand the constraints on which you are working with
here, but please, let me do a tiny rant about how you all are doing
"extra work" with regards to the creation of sysfs files:

On Tue, Feb 19, 2019 at 04:58:11PM +0100, speck for Peter Zijlstra wrote:
> @@ -4123,8 +4179,11 @@ static struct attribute *intel_pmu_caps_
>         NULL
>  };
>  
> +DEVICE_BOOL_ATTR(allow_tsx_force_abort, 0644, allow_tsx_force_abort);
> +
>  static struct attribute *intel_pmu_attrs[] = {
>  	&dev_attr_freeze_on_smi.attr,
> +	NULL, /* &dev_attr_allow_tsx_force_abort.attr.attr */
>  	NULL,
>  };
>  
> @@ -4623,6 +4682,15 @@ __init int intel_pmu_init(void)
>  		tsx_attr = hsw_tsx_events_attrs;
>  		intel_pmu_pebs_data_source_skl(
>  			boot_cpu_data.x86_model == INTEL_FAM6_SKYLAKE_X);
> +
> +		if (boot_cpu_has(X86_FEATURE_TSX_FORCE_ABORT)) {
> +			x86_pmu.flags |= PMU_FL_TFA;
> +			x86_pmu.get_event_constraints = skl_get_event_constraints;
> +			x86_pmu.enable_all = intel_skl_pmu_enable_all;
> +			x86_pmu.commit_scheduling = intel_skl_commit_scheduling;
> +			intel_pmu_attrs[1] = &dev_attr_allow_tsx_force_abort.attr.attr;
> +		}
> +
>  		pr_cont("Skylake events, ");
>  		name = "skylake";
>  		break;

sysfs files have the ability to be "shown" or not, on their own.  There
is a "is_visable()" callback for every sysfs group.

This allows you to set a whole bunch of groups to a device, and then,
when the device is created, the kobject core will call back to you and
ask "should I show this file?"  At that point in time, you can do your
check for "boot_cpu_has(...)" and the like to see if you really should
be showing the file or not.

This saves you all the horrid mess of the merge_attr() function, trying
to overload NULL pointers here in this attribute list, and other such
hacks.

I guess no one stopped to think why _only_ the perf cpu code has a
function like merge_attr(), and how the rest of the kernel could get
away without needing a hack like that :(

Anyway, rant over.  I'll see if I can squeze this into the 4.9.y tree as
it was before the major revamp of the perf cpu sysfs files.

In the future, someone should just switch all of this to attribute
groups, like the rest of the kernel uses, so no such crazyness is
needed.

thanks,

greg k-h