linux-kernel - Re: [PATCH v3 4/4] 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: <36C0AD99-0D75-40D4-B704-507A222AEB81@javigon.com>
Date:   Wed, 8 Jul 2020 18:08:12 +0200
From:   Javier González <javier@...igon.com>
To:     Jens Axboe <axboe@...nel.dk>
Cc:     Matthew Wilcox <willy@...radead.org>,
        Kanchan Joshi <joshi.k@...sung.com>, viro@...iv.linux.org.uk,
        bcrl@...ck.org, hch@...radead.org, damien.lemoal@....com,
        asml.silence@...il.com, linux-fsdevel@...r.kernel.org,
        mb@...htnvm.io, linux-kernel@...r.kernel.org, linux-aio@...ck.org,
        io-uring@...r.kernel.org, linux-block@...r.kernel.org,
        Selvakumar S <selvakuma.s1@...sung.com>,
        Nitesh Shetty <nj.shetty@...sung.com>,
        Javier Gonzalez <javier.gonz@...sung.com>
Subject: Re: [PATCH v3 4/4] io_uring: add support for zone-append


> On 8 Jul 2020, at 17.06, Jens Axboe <axboe@...nel.dk> wrote:
> 
> On 7/8/20 9:02 AM, Matthew Wilcox wrote:
>>> On Wed, Jul 08, 2020 at 08:59:50AM -0600, Jens Axboe wrote:
>>> On 7/8/20 8:58 AM, Matthew Wilcox wrote:
>>>> On Wed, Jul 08, 2020 at 08:54:07AM -0600, Jens Axboe wrote:
>>>>> On 7/8/20 6:58 AM, Kanchan Joshi wrote:
>>>>>>>> +#define IOCB_NO_CMPL        (15 << 28)
>>>>>>>> 
>>>>>>>> struct kiocb {
>>>>>>>> [...]
>>>>>>>> -    void (*ki_complete)(struct kiocb *iocb, long ret, long ret2);
>>>>>>>> +    loff_t __user *ki_uposp;
>>>>>>>> -    int            ki_flags;
>>>>>>>> +    unsigned int        ki_flags;
>>>>>>>> 
>>>>>>>> +typedef void ki_cmpl(struct kiocb *, long ret, long ret2);
>>>>>>>> +static ki_cmpl * const ki_cmpls[15];
>>>>>>>> 
>>>>>>>> +void ki_complete(struct kiocb *iocb, long ret, long ret2)
>>>>>>>> +{
>>>>>>>> +    unsigned int id = iocb->ki_flags >> 28;
>>>>>>>> +
>>>>>>>> +    if (id < 15)
>>>>>>>> +        ki_cmpls[id](iocb, ret, ret2);
>>>>>>>> +}
>>>>>>>> 
>>>>>>>> +int kiocb_cmpl_register(void (*cb)(struct kiocb *, long, long))
>>>>>>>> +{
>>>>>>>> +    for (i = 0; i < 15; i++) {
>>>>>>>> +        if (ki_cmpls[id])
>>>>>>>> +            continue;
>>>>>>>> +        ki_cmpls[id] = cb;
>>>>>>>> +        return id;
>>>>>>>> +    }
>>>>>>>> +    WARN();
>>>>>>>> +    return -1;
>>>>>>>> +}
>>>>>>> 
>>>>>>> That could work, we don't really have a lot of different completion
>>>>>>> types in the kernel.
>>>>>> 
>>>>>> Thanks, this looks sorted.
>>>>> 
>>>>> Not really, someone still needs to do that work. I took a quick look, and
>>>>> most of it looks straight forward. The only potential complication is
>>>>> ocfs2, which does a swap of the completion for the kiocb. That would just
>>>>> turn into an upper flag swap. And potential sync kiocb with NULL
>>>>> ki_complete. The latter should be fine, I think we just need to reserve
>>>>> completion nr 0 for being that.
>>>> 
>>>> I was reserving completion 15 for that ;-)
>>>> 
>>>> +#define IOCB_NO_CMPL        (15 << 28)
>>>> ...
>>>> +    if (id < 15)
>>>> +        ki_cmpls[id](iocb, ret, ret2);
>>>> 
>>>> Saves us one pointer in the array ...
>>> 
>>> That works. Are you going to turn this into an actual series of patches,
>>> adding the functionality and converting users?
>> 
>> I was under the impression Kanchan was going to do that, but I can run it
>> off quickly ...
> 
> I just wanted to get clarification there, because to me it sounded like
> you expected Kanchan to do it, and Kanchan assuming it "was sorted". I'd
> consider that a prerequisite for the append series as far as io_uring is
> concerned, hence _someone_ needs to actually do it ;-)
> 

I believe Kanchan meant that now the trade-off we were asking to clear out is sorted. 

We will send a new version shortly for the current functionality - we can see what we are missing on when the uring interface is clear. 

We really want this to be stable as a lot of other things are depending on this (e.g., fio patches)

Javier