From: Naman Jain <[email protected]> Sent: Thursday, July 24, 2025
10:54 PM
>
> On 7/25/2025 8:52 AM, Michael Kelley wrote:
> > From: Naman Jain <[email protected]> Sent: Thursday, July 24,
> > 2025 1:26 AM
> >>
[snip]
> >> +
> >> +static int mshv_vtl_sint_ioctl_set_eventfd(struct mshv_vtl_set_eventfd
> >> __user *arg)
> >> +{
> >> + struct mshv_vtl_set_eventfd set_eventfd;
> >> + struct eventfd_ctx *eventfd, *old_eventfd;
> >> +
> >> + if (copy_from_user(&set_eventfd, arg, sizeof(set_eventfd)))
> >> + return -EFAULT;
> >> + if (set_eventfd.flag >= HV_EVENT_FLAGS_COUNT)
> >> + return -EINVAL;
> >> +
> >> + eventfd = NULL;
> >> + if (set_eventfd.fd >= 0) {
> >> + eventfd = eventfd_ctx_fdget(set_eventfd.fd);
> >> + if (IS_ERR(eventfd))
> >> + return PTR_ERR(eventfd);
> >> + }
> >> +
> >> + guard(mutex)(&flag_lock);
> >> + old_eventfd = READ_ONCE(flag_eventfds[set_eventfd.flag]);
> >> + WRITE_ONCE(flag_eventfds[set_eventfd.flag], eventfd);
> >> +
> >> + if (old_eventfd) {
> >> + synchronize_rcu();
> >> + eventfd_ctx_put(old_eventfd);
> >
> > Again, I wonder if is OK to do eventfd_ctx_put() while holding
> > flag_lock, since the use of guard() changes the scope of the lock
> > compared with the previous version of this patch.
> >
>
> I didn't find eventfd_ctx_put() to be a blocking operation, so I thought
> of keeping guard() here. Although, synchronize_rcu() is a blocking
> operation. Please advise, I am Ok with removing the guard, as the lock
> is just being used here, and automatic cleanup should not be an issue
> here.
Yes, I think you are right. I saw the kref_put() and was unsure what
would be called if the object was freed. But the "free" function is
right there staring at me. :-) All it does is ida_free() and kfree(),
both of which would be safe.
You should be good keeping the guard().
Michael
>
>
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
