Re: [PATCH 3/8] fs: add file_operations->uring_cmd()

[Jens Axboe <axboe@kernel.dk>]

On 3/17/21 11:38 PM, Christoph Hellwig wrote:
> On Wed, Mar 17, 2021 at 04:10:22PM -0600, Jens Axboe wrote:
>> This is a file private handler, similar to ioctls but hopefully a lot
>> more sane and useful.
> 
> I really hate defining the interface in terms of io_uring.  This really
> is nothing but an async ioctl.

Sure, it's generic, could potentially use any transport. But the way it's
currently setup is using an io_uring transport and embedded command.

>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>> index ec8f3ddf4a6a..009abc668987 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -1884,6 +1884,15 @@ struct dir_context {
>>  #define REMAP_FILE_ADVISORY		(REMAP_FILE_CAN_SHORTEN)
>>  
>>  struct iov_iter;
>> +struct io_uring_cmd;
>> +
>> +/*
>> + * f_op->uring_cmd() issue flags
>> + */
>> +enum io_uring_cmd_flags {
>> +	IO_URING_F_NONBLOCK		= 1,
>> +	IO_URING_F_COMPLETE_DEFER	= 2,
>> +};
> 
> I'm a little worried about exposing a complete io_uring specific
> concept like IO_URING_F_COMPLETE_DEFER to random drivers.  This
> needs to be better encapsulated.

Agree.

>>  struct file_operations {
>>  	struct module *owner;
>> @@ -1925,6 +1934,8 @@ struct file_operations {
>>  				   struct file *file_out, loff_t pos_out,
>>  				   loff_t len, unsigned int remap_flags);
>>  	int (*fadvise)(struct file *, loff_t, loff_t, int);
>> +
>> +	int (*uring_cmd)(struct io_uring_cmd *, enum io_uring_cmd_flags);
> 
> As of this patch io_uring_cmd is still a private structure.  In general
> I'm not sure this patch makes much sense on its own either.

Might indeed just fold it in or reshuffle, I'll take a look.

-- 
Jens Axboe