Re: [PATCH 06/19] io_uring: add fsync support

[Jens Axboe <axboe@kernel.dk>]

On 2/8/19 3:36 PM, Jann Horn wrote:
> On Fri, Feb 8, 2019 at 6:34 PM Jens Axboe <axboe@kernel.dk> wrote:
>> From: Christoph Hellwig <hch@lst.de>
>>
>> Add a new fsync opcode, which either syncs a range if one is passed,
>> or the whole file if the offset and length fields are both cleared
>> to zero.  A flag is provided to use fdatasync semantics, that is only
>> force out metadata which is required to retrieve the file data, but
>> not others like metadata.
>>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>> ---
> [...]
>> +static int io_fsync(struct io_kiocb *req, const struct io_uring_sqe *sqe,
>> +                   bool force_nonblock)
>> +{
>> +       struct io_ring_ctx *ctx = req->ctx;
>> +       loff_t sqe_off = READ_ONCE(sqe->off);
>> +       loff_t sqe_len = READ_ONCE(sqe->len);
>> +       loff_t end = sqe_off + sqe_len;
>> +       unsigned fsync_flags;
>> +       struct file *file;
>> +       int ret, fd;
>> +
>> +       /* fsync always requires a blocking context */
>> +       if (force_nonblock)
>> +               return -EAGAIN;
>> +
>> +       if (unlikely(sqe->addr || sqe->ioprio))
>> +               return -EINVAL;
>> +
>> +       fsync_flags = READ_ONCE(sqe->fsync_flags);
>> +       if (unlikely(fsync_flags & ~IORING_FSYNC_DATASYNC))
>> +               return -EINVAL;
>> +
>> +       fd = READ_ONCE(sqe->fd);
>> +       file = fget(fd);
> 
> This always runs on the workqueue, right? Is it possible to call
> fget() on a workqueue?

Oops yes, I've now split that into a io_prep_fsync() and io_fsync()
part so that it works correctly with this new method. Also added a
liburing test for this.

-- 
Jens Axboe