public inbox for [email protected]
 help / color / mirror / Atom feed
From: Gabriel Krisman Bertazi <[email protected]>
To: Jens Axboe <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH 1/6] io_uring: expand main struct io_kiocb flags to 64-bits
Date: Thu, 08 Feb 2024 15:52:23 -0500	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]> (Jens Axboe's message of "Thu, 8 Feb 2024 13:22:35 -0700")

Jens Axboe <[email protected]> writes:

> On 2/8/24 1:08 PM, Gabriel Krisman Bertazi wrote:
>> Jens Axboe <[email protected]> writes:
>> 
>> 
>>> -	TP_printk("ring %p, request %p, user_data 0x%llx, opcode %s, flags 0x%x, %s queue, work %p",
>>> +	TP_printk("ring %p, request %p, user_data 0x%llx, opcode %s, flags 0x%lx, %s queue, work %p",
>>>  		__entry->ctx, __entry->req, __entry->user_data,
>>> -		__get_str(op_str),
>>> -		__entry->flags, __entry->rw ? "hashed" : "normal", __entry->work)
>>> +		__get_str(op_str), (long) __entry->flags,
>> 
>> Hi Jens,
>> 
>> Minor, but on 32-bit kernel the cast is wrong since
>> sizeof(long)==4. Afaik, io_uring still builds on 32-bit archs.
>> 
>> If you use (unsigned long long), it will be 64 bit anywhere.
>
> Ah thanks, I'll make that edit.
>
>>> @@ -2171,7 +2171,8 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req,
>>>  	/* req is partially pre-initialised, see io_preinit_req() */
>>>  	req->opcode = opcode = READ_ONCE(sqe->opcode);
>>>  	/* same numerical values with corresponding REQ_F_*, safe to copy */
>>> -	req->flags = sqe_flags = READ_ONCE(sqe->flags);
>>> +	sqe_flags = READ_ONCE(sqe->flags);
>> 
>> Did you consider that READ_ONCE won't protect from load tearing the
>> userspace value in 32-bit architectures? It builds silently, though, and
>> I suspect it is mostly fine in the current code, but might become a bug
>> eventually.
>
> sqe->flags is just a byte, so no tearing is possible here. The only
> thing that changed type is req->flags.

You're right, of course. I confused the source of the read with struct
io_kiocb.

-- 
Gabriel Krisman Bertazi

  reply	other threads:[~2024-02-08 20:52 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-07 17:17 [PATCHSET v2 0/6] Misc cleanups / optimizations Jens Axboe
2024-02-07 17:17 ` [PATCH 1/6] io_uring: expand main struct io_kiocb flags to 64-bits Jens Axboe
2024-02-08 20:08   ` Gabriel Krisman Bertazi
2024-02-08 20:22     ` Jens Axboe
2024-02-08 20:52       ` Gabriel Krisman Bertazi [this message]
2024-02-07 17:17 ` [PATCH 2/6] io_uring/cancel: don't default to setting req->work.cancel_seq Jens Axboe
2024-02-07 17:17 ` [PATCH 3/6] io_uring: add io_file_can_poll() helper Jens Axboe
2024-02-07 17:17 ` [PATCH 4/6] io_uring: mark the need to lock/unlock the ring as unlikely Jens Axboe
2024-02-07 17:17 ` [PATCH 5/6] io_uring: cleanup io_req_complete_post() Jens Axboe
2024-02-07 17:17 ` [PATCH 6/6] io_uring/rw: remove dead file == NULL check Jens Axboe
  -- strict thread matches above, loose matches on Subject: below --
2024-02-06 16:22 [PATCHSET next 0/6] Misc cleanups / optimizations Jens Axboe
2024-02-06 16:22 ` [PATCH 1/6] io_uring: expand main struct io_kiocb flags to 64-bits Jens Axboe
2024-02-06 22:58   ` Jens Axboe
2024-02-07  0:43   ` Pavel Begunkov
2024-02-07  2:18     ` Jens Axboe
2024-02-07  3:22       ` Pavel Begunkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox