Re: [PATCH v3 8/8] drm/xe/gt_throttle: Avoid TOCTOU when monitoring reasons

[Raag Jadav <raag.jadav@intel.com>]

On Wed, Oct 29, 2025 at 04:45:10PM -0700, Lucas De Marchi wrote:
> It's currently not possible to safely monitor if there's throttling
> happening and what are the reasons. The approach of reading the status
> and then reading the reasons is not reliable as by the time sysadmin
> reads the reason, the throttling could not be happening anymore.
> 
> Previous tentative to fix that[1] was breaking the ABI and potentially
> sysadmin's scripts. This takes a different approach of adding and
> documenting the additional attribute. It's still valuable, though
> redundant, to provide the simpler 0/1 interface.
> 
> In order to avoid userspace knowledge on the bitmask meaning and to be
> able to maintain the kernel side in sync with possible changes in
> future, just walk the attribute group and check what are the masks that
> match the value read.
> 
> [1] https://lore.kernel.org/intel-xe/20241025092238.167042-1-raag.jadav@intel.com/

...

> +static const struct attribute_group *get_platform_throttle_group(struct xe_device *xe);
> +
> +static ssize_t status_reasons_show(struct kobject *kobj,
> +				   struct kobj_attribute *attr, char *buff)
> +{
> +	struct xe_gt *gt = throttle_to_gt(kobj);
> +	struct xe_device *xe = gt_to_xe(gt);
> +	const struct attribute_group *group;
> +	struct attribute **pother;
> +	ssize_t ret = 0;
> +	u32 reasons;
> +
> +	reasons = xe_gt_throttle_get_limit_reasons(gt);
> +	group = get_platform_throttle_group(xe);
> +
> +	for (pother = group->attrs; *pother; pother++) {
> +		struct kobj_attribute *kattr = container_of(*pother, struct kobj_attribute, attr);
> +		struct throttle_attribute *other_ta = kobj_attribute_to_throttle(kattr);
> +
> +		if (other_ta->mask != U32_MAX && reasons & other_ta->mask)
> +			ret += sysfs_emit_at(buff, ret, "%s ", (*pother)->name);

Much better.

> +	}
> +
> +	/* Drop extra space from last iteration above */
> +	if (ret)
> +		ret--;
> +
> +	ret += sysfs_emit_at(buff, ret, "\n");

I went through the documentation again and I couldn't find any rules
related to empty files or whether it is allowed (just thinking out
loud about no throttling cases).

Thoughts?

Raag