On 2023/12/1 23:24, Jason Gunthorpe wrote:
On Thu, Oct 26, 2023 at 10:49:28AM +0800, Lu Baolu wrote:

+static ssize_t hwpt_fault_fops_write(struct file *filep,
+                                    const char __user *buf,
+                                    size_t count, loff_t *ppos)
+{
+       size_t response_size = sizeof(struct iommu_hwpt_page_response);
+       struct hw_pgtable_fault *fault = filep->private_data;
+       struct iommu_hwpt_page_response response;
+       struct iommufd_hw_pagetable *hwpt;
+       struct iopf_group *iter, *group;
+       struct iommufd_device *idev;
+       size_t done = 0;
+       int rc = 0;
+
+       if (*ppos || count % response_size)
+               return -ESPIPE;
+
+       mutex_lock(&fault->mutex);
+       while (!list_empty(&fault->response) && count > done) {
+               rc = copy_from_user(&response, buf + done, response_size);
+               if (rc)
+                       break;
+
+               /* Get the device that this response targets at. */
+               idev = container_of(iommufd_get_object(fault->ictx,
+                                                      response.dev_id,
+                                                      IOMMUFD_OBJ_DEVICE),
+                                   struct iommufd_device, obj);
+               if (IS_ERR(idev)) {
+                       rc = PTR_ERR(idev);
+                       break;
+               }

See here it might be better to have a per-fd list of outstanding
faults per-fd and then the cookie would just index that list, then you
get everything in one shot instead of having to do a xarray looking
and then a linear list search

Yours is more efficient. I will do it that way in the next version.


+static const struct file_operations hwpt_fault_fops = {
+       .owner          = THIS_MODULE,
+       .read           = hwpt_fault_fops_read,
+       .write          = hwpt_fault_fops_write,
+};

nonseekable_open() behavior should be integrated into this

Sure.


+static int hw_pagetable_get_fault_fd(struct hw_pgtable_fault *fault)
+{
+       struct file *filep;
+       int fdno;
+
+       fdno = get_unused_fd_flags(O_CLOEXEC);
+       if (fdno < 0)
+               return fdno;
+
+       filep = anon_inode_getfile("[iommufd-pgfault]", &hwpt_fault_fops,
+                                  fault, O_RDWR);
+       if (IS_ERR(filep)) {
+               put_unused_fd(fdno);
+               return PTR_ERR(filep);
+       }
+
+       fd_install(fdno, filep);
+       fault->fault_file = filep;
+       fault->fault_fd = fdno;

fd_install must be the very last thing before returning success from a
system call because we cannot undo it.

Yes.


There are other failure paths before here and the final return

Jason

Best regards,
baolu


Reply via email to