Messages in this thread | | | Subject | Re: [PATCH 1/4] Vectorize aio_read/aio_write methods | From | Badari Pulavarty <> | Date | Thu, 11 May 2006 12:07:44 -0700 |
| |
On Thu, 2006-05-11 at 11:47 -0700, Andrew Morton wrote: > Badari Pulavarty <pbadari@us.ibm.com> wrote: > > > > static ssize_t ep_aio_read_retry(struct kiocb *iocb) > > { > > struct kiocb_priv *priv = iocb->private; > > - ssize_t status = priv->actual; > > + ssize_t len, total; > > > > /* we "retry" to get the right mm context for this: */ > > - status = copy_to_user(priv->ubuf, priv->buf, priv->actual); > > - if (unlikely(0 != status)) > > - status = -EFAULT; > > - else > > - status = priv->actual; > > + > > + /* copy stuff into user buffers */ > > + total = priv->actual; > > + len = 0; > > + for (i=0; i < priv->count; i++) { > > + ssize_t this = min(priv->iv[i].iov_len, total); > > + > > + if (copy_to_user(priv->iv[i].iov_buf, priv->buf, this)) > > + break; > > + > > + total -= this; > > + len += this; > > + if (total <= 0) > > + break; > > + } > > + > > + if (unlikely(len == 0)) > > + len = -EFAULT; > > This is still wrong, isn't it? Or am I looking at the same patch? > > There's no way in which `total' can go negative, so it'd be nicer to just > test it for equality with zero. Because if it goes unexpectedly negative, > we _want_ the kernel to malfunction, rather than mysteriously covering > things up. > > The final test there should be > > if (unlikely(total != 0)) > > yes?
No. The original check is correct - we want to return EFAULT if copy_to_user() failed and we haven't copied anything so far. If we copied anything so far, we should return, that many bytes. (like short-io).
Thanks, Badari
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |