linux-kernel - Re: [PATCH 3/3] io_uring: add support for zone-append

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CY4PR04MB3751D5D6AFB0DA7B8A2DFF61E79B0@CY4PR04MB3751.namprd04.prod.outlook.com>
Date:   Thu, 18 Jun 2020 08:47:29 +0000
From:   Damien Le Moal <Damien.LeMoal@....com>
To:     "javier.gonz@...sung.com" <javier@...igon.com>
CC:     Kanchan Joshi <joshi.k@...sung.com>,
        "axboe@...nel.dk" <axboe@...nel.dk>,
        "viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
        "bcrl@...ck.org" <bcrl@...ck.org>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-aio@...ck.org" <linux-aio@...ck.org>,
        "io-uring@...r.kernel.org" <io-uring@...r.kernel.org>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        "selvakuma.s1@...sung.com" <selvakuma.s1@...sung.com>,
        "nj.shetty@...sung.com" <nj.shetty@...sung.com>
Subject: Re: [PATCH 3/3] io_uring: add support for zone-append

On 2020/06/18 17:35, javier.gonz@...sung.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@...sung.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@...sung.com>
>>> Signed-off-by: Kanchan Joshi <joshi.k@...sung.com>
>>> Signed-off-by: Nitesh Shetty <nj.shetty@...sung.com>
>>> Signed-off-by: Javier Gonzalez <javier.gonz@...sung.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.

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).


> 
> Javier
> 


-- 
Damien Le Moal
Western Digital Research