[lkml]   [2006]   [Aug]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [2.6.18-rc* PATCH RFC]: Correct ambiguous errno of aio

Sorry, we shouldn't merge this patch in its current form.

> In the current implementation of AIO, for the operation IOCB_CMD_FDSYNC
> and IOCB_CMD_FSYNC, the returned errno is -EINVAL although the kernel
> does know them, I think the correct errno should be -EOPNOTSUPP which
> means they aren't be implemented or supported.

Like it or not, the sys_io_submit() interface returns -EINVAL when the
file descriptor doesn't support the requested command. Changing the
binary interface is a big deal and should not be done lightly. What is
the motivation for making this change?

Even if we decided to, we'd want to do it for all the commands. This
patch only addresses F{D,}SYNC. All the other commands would still
return -EINVAL if the descriptor doesn't have the corresponding ->aio_
method, leaving userspace do deal with more complexity.

> -static ssize_t aio_fdsync(struct kiocb *iocb)
> -{
> - struct file *file = iocb->ki_filp;
> - ssize_t ret = -EINVAL;
> -
> - if (file->f_op->aio_fsync)
> - ret = file->f_op->aio_fsync(iocb, 1);
> - return ret;
> -}
> -
> static ssize_t aio_fsync(struct kiocb *iocb)
> {
> struct file *file = iocb->ki_filp;
> - ssize_t ret = -EINVAL;
> + ssize_t ret = -EOPNOTSUPP;
> if (file->f_op->aio_fsync)
> ret = file->f_op->aio_fsync(iocb, 0);

> - ret = -EINVAL;
> + ret = -EOPNOTSUPP;
> if (file->f_op->aio_fsync)
> - kiocb->ki_retry = aio_fdsync;
> + kiocb->ki_retry = aio_fsync;

Hmm, your most recent patch didn't mention this aio_f{d,}sync() change
though the earlier one did. Please make sure all patch submissions have
complete descriptions.

These calls are not the same, notice that they differ in the second
argument to their ->aio_fsync() calls. Cleaning up the ->aio_fsync()
interface might well be reasonable. Missing that subtle difference
suggests that it should be more clear and there are precisely zero
merged ->aio_fsync() users. But that kind of cleanup belongs in a
separate patch with its own justification.

Are you working with an ->aio_fsync() user that might be merged?

- z
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at
Please read the FAQ at

 \ /
  Last update: 2006-08-29 20:35    [W:0.060 / U:3.140 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site