On 9/23/2021 6:02 PM, Thomas Gleixner wrote:

On Mon, Sep 13 2021 at 13:01, Sohil Mehta wrote:

quoted

The user interrupt MSRs and the user interrupt state is task specific.
During task fork and exit clear the task state, clear the MSRs and
dereference the shared resources.

Some of the memory resources like the UPID are referenced in the file
descriptor and could be in use while the uintr_fd is still valid.
Instead of freeing up  the UPID just dereference it.

Derefencing the UPID, i.e. accessing task->upid->foo helps in which way?

You want to drop the reference count I assume. Then please write that
so.

Ah! Not sure how I associated dereference to dropping the reference. 
Will update this.

quoted

@@ -260,6 +260,7 @@ int fpu_clone(struct task_struct *dst)
  {
  	struct fpu *src_fpu = &current->thread.fpu;
  	struct fpu *dst_fpu = &dst->thread.fpu;
+	struct uintr_state *uintr_state;
  
  	/* The new task's FPU state cannot be valid in the hardware. */
  	dst_fpu->last_cpu = -1;

@@ -284,6 +285,14 @@ int fpu_clone(struct task_struct *dst)
  
  	else
  		save_fpregs_to_fpstate(dst_fpu);
+
+	/* UINTR state is not expected to be inherited (in the current design). */
+	if (static_cpu_has(X86_FEATURE_UINTR)) {
+		uintr_state = get_xsave_addr(&dst_fpu->state.xsave, XFEATURE_UINTR);
+		if (uintr_state)
+			memset(uintr_state, 0, sizeof(*uintr_state));
+	}

1) If the FPU registers are up to date then this can be completely
    avoided by excluding the UINTR component from XSAVES

You mentioned this in the other thread that the UINTR state must be 
invalidated during fpu_clone().

I am not sure if understand all the nuances here. Your suggestion seems 
valid to me. I'll have to think more about this.

2) If the task never used that muck then UINTR is in init state and
    clearing that memory is a redunant exercise because it has been
    cleared already

Yes. I'll add a check for that.

quoted

+ * exit_thread() can happen in current context when the current thread is
+ * exiting or it can happen for a new thread that is being created.

A right that makes sense. If a new thread is created then it can call
exit_thread(), right?

What I meant here is that exit_thread() can also be called during 
copy_process() if it runs into an issue.

bad_fork_cleanup_thread:

     exit_thread();

In this case is_uintr_receiver() will fail. I'll update the comments to 
reflect that.

quoted

+ * For new threads is_uintr_receiver() should fail.

Should fail?

Thanks,

Sohil