[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);

    > case IOCB_CMD_FDSYNC:
    > - 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.023 / U:9.892 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site