> From: Jason Gunthorpe <jgg@xxxxxxxx>
> Sent: Wednesday, June 12, 2024 9:24 PM
>
> On Fri, Jun 07, 2024 at 09:17:28AM +0000, Tian, Kevin wrote:
> > > From: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> > > Sent: Monday, May 27, 2024 12:05 PM
> > >
> > > +static ssize_t iommufd_fault_fops_read(struct file *filep, char __user
> *buf,
> > > + size_t count, loff_t *ppos)
> > > +{
> > > + size_t fault_size = sizeof(struct iommu_hwpt_pgfault);
> > > + struct iommufd_fault *fault = filep->private_data;
> > > + struct iommu_hwpt_pgfault data;
> > > + struct iommufd_device *idev;
> > > + struct iopf_group *group;
> > > + struct iopf_fault *iopf;
> > > + size_t done = 0;
> > > + int rc = 0;
> > > +
> > > + if (*ppos || count % fault_size)
> > > + return -ESPIPE;
> >
> > the man page says:
> >
> > "If count is zero, read() returns zero and has no other results."
>
> The above does that? 0 % X == 0
but if *ppos is non-zero then an error will be returned even if count==0.
but seems I looked at an old man page on an old system. A newer one
says:
If count is zero, read() may detect the errors described below.
In the absence of any errors, or if read() does not check for
errors, a read() with a count of 0 returns zero and has no other
effects.
so above should be fine. 😊
> > > + xa_erase(&fault->response, group->cookie);
> > > + break;
> > > + }
> > > + done += fault_size;
> > > + }
> > > +
> > > + list_del(&group->node);
> > > + }
> > > + mutex_unlock(&fault->mutex);
> > > +
> > > + return done == 0 ? rc : done;
> >
> > again this doesn't match the manual:
> >
> > "On error, -1 is returned, and errno is set appropriately. "
> >
> > it doesn't matter whether 'done' is 0.
>
> It is setup so that once the list_del() below happens it is guarenteed
> that the system call will return a positive result so that the
> list_del'd items are always returned to userspace.
>
> If we hit any fault here on the Nth item we should still return the
> prior items and ignore the fault.
>
> If we hit a fault on the first item then we should return the fault.
>
yes that does make sense. I just didn't find such statement in the
man page which simply said to return -1 on error. The number of
bytes is returned only on success.
RETURN VALUE
On success, the number of bytes read is returned (zero indicates
end of file), and the file position is advanced by this number.
It is not an error if this number is smaller than the number of
bytes requested; this may happen for example because fewer bytes
are actually available right now (maybe because we were close to
end-of-file, or because we are reading from a pipe, or from a
terminal), or because read() was interrupted by a signal. See
also NOTES.
On error, -1 is returned, and errno is set to indicate the error.
In this case, it is left unspecified whether the file position
(if any) changes.
