lkml.org 
[lkml]   [2020]   [Jun]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 3/3] io_uring: add support for zone-append
On 18.06.2020 08:47, Damien Le Moal wrote:
>On 2020/06/18 17:35, javier.gonz@samsung.com wrote:
>> On 18.06.2020 07:39, Damien Le Moal wrote:
>>> On 2020/06/18 2:27, Kanchan Joshi wrote:
>>>> From: Selvakumar S <selvakuma.s1@samsung.com>
>>>>
>>>> Introduce three new opcodes for zone-append -
>>>>
>>>> IORING_OP_ZONE_APPEND : non-vectord, similiar to IORING_OP_WRITE
>>>> IORING_OP_ZONE_APPENDV : vectored, similar to IORING_OP_WRITEV
>>>> IORING_OP_ZONE_APPEND_FIXED : append using fixed-buffers
>>>>
>>>> Repurpose cqe->flags to return zone-relative offset.
>>>>
>>>> Signed-off-by: SelvaKumar S <selvakuma.s1@samsung.com>
>>>> Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>
>>>> Signed-off-by: Nitesh Shetty <nj.shetty@samsung.com>
>>>> Signed-off-by: Javier Gonzalez <javier.gonz@samsung.com>
>>>> ---
>>>> fs/io_uring.c | 72 +++++++++++++++++++++++++++++++++++++++++--
>>>> include/uapi/linux/io_uring.h | 8 ++++-
>>>> 2 files changed, 77 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>>>> index 155f3d8..c14c873 100644
>>>> --- a/fs/io_uring.c
>>>> +++ b/fs/io_uring.c
>>>> @@ -649,6 +649,10 @@ struct io_kiocb {
>>>> unsigned long fsize;
>>>> u64 user_data;
>>>> u32 result;
>>>> +#ifdef CONFIG_BLK_DEV_ZONED
>>>> + /* zone-relative offset for append, in bytes */
>>>> + u32 append_offset;
>>>
>>> this can overflow. u64 is needed.
>>
>> We chose to do it this way to start with because struct io_uring_cqe
>> only has space for u32 when we reuse the flags.
>>
>> We can of course create a new cqe structure, but that will come with
>> larger changes to io_uring for supporting append.
>>
>> Do you believe this is a better approach?
>
>The problem is that zone size are 32 bits in the kernel, as a number of sectors.
>So any device that has a zone size smaller or equal to 2^31 512B sectors can be
>accepted. Using a zone relative offset in bytes for returning zone append result
>is OK-ish, but to match the kernel supported range of possible zone size, you
>need 31+9 bits... 32 does not cut it.

Agree. Our initial assumption was that u32 would cover current zone size
requirements, but if this is a no-go, we will take the longer path.

>
>Since you need a 64-bit sized result, I would also prefer that you drop the zone
>relative offset as a result and return the absolute offset instead. That makes
>life easier for the applications since the zone append requests also must use
>absolute offsets for zone start. An absolute offset as a result becomes
>consistent with that and all other read/write system calls that all use absolute
>offsets (seek() is the only one that I know of that can use a relative offset,
>but that is not an IO system call).

Agree. Using relative offsets was a product of reusing the existing u32.
If we move to u64, there is no need to do an extra transformation.

Thanks Damien!
Javier

\
 
 \ /
  Last update: 2020-06-18 11:12    [W:0.227 / U:0.148 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site