Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [RFCv2 03/10] x86: Introduce userspace API to handle per-thread features | Date | Thu, 12 May 2022 14:02:50 +0200 |
| |
On Wed, May 11 2022 at 05:27, Kirill A. Shutemov wrote: > Add three new arch_prctl() handles: > > +static long thread_feature_prctl(struct task_struct *task, int option, > + unsigned long features)
Bah. I really hate the task pointer on all these x86 prctls. @task must always be current, so this @task argument is just confusion.
> +{ > + const unsigned long known_features = 0; > + > + if (features & ~known_features) > + return -EINVAL;
This implementation allows to task->read features_[locked] with @features == 0. That should be documented somewhere.
> + if (option == ARCH_THREAD_FEATURE_LOCK) { > + task->thread.features_locked |= features; > + return task->thread.features_locked; > + }
> + /* Do not allow to change locked features */ > + if (features & task->thread.features_locked) > + return -EPERM; > + > + if (option == ARCH_THREAD_FEATURE_DISABLE) { > + task->thread.features &= ~features; > + goto out; > + } > + > + /* Handle ARCH_THREAD_FEATURE_ENABLE */ > + > + task->thread.features |= features; > +out: > + return task->thread.features;
Eyes bleed.
if (option == ARCH_THREAD_FEATURE_ENABLE) task->thread.features |= features; else task->thread.features &= ~features;
return task->thread.features;
It's bloody obvious from the code that the counterpart of enable is disable, no? So neither the goto nor the 'comment the obvious' is useful in any way.
Thanks,
tglx
| |