From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qt1-f169.google.com (mail-qt1-f169.google.com [209.85.160.169]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 962BD2777F3 for ; Thu, 4 Dec 2025 19:22:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.160.169 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764876168; cv=none; b=Iumh9Q6bZqTkOFqcOv3AYcM3o2qM4HPsDyOmj8+i8hWVMg+a+Mu3Ef4cYR5scGQY+i9h/sEicFPAJOOLLGOLvlMGnrYisn2W6TC/xx5WZ186krAw3Y/H0ttXSADcspNQEMz8NI/WoRWdDt3YvQrMmy1mTdyx1aj8oGvSaYtY820= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764876168; c=relaxed/simple; bh=mGOfBEPmG1Q3Rp35eCArNKy6bUbynQIeFUMWEaU4hYc=; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type; b=bWm+imSsloY3phRgiS6g1APD5vzF6B+VqLak91cIcomHWC5deEclSAXL54xMAsiaRMkNA6MF1K04AybPnfO8/MGjCYM1IAMbN12b6oRohEef/g4Adj/S/hSxCyigbvELDjw5CojMVJ2fo0Tp8t53ZCTuRqgn4NvwvrUe4Tab+DI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=EPzlYjKf; arc=none smtp.client-ip=209.85.160.169 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="EPzlYjKf" Received: by mail-qt1-f169.google.com with SMTP id d75a77b69052e-4ee158187aaso12652151cf.0 for ; Thu, 04 Dec 2025 11:22:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1764876165; x=1765480965; darn=vger.kernel.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=qRUfxBLVaD2BUiIJM9PrjNYt0vrl8q05tvZ2QQvcI7c=; b=EPzlYjKfs1hXLm78hOIohNcs6H7/TkDYvQaGz4hwWojD2/evPBzFcllL7ijTarSVm0 Cn2J/tcUkDexJ8lj+S9mBvB4bKk6cPcWyr9tcOBke4Jwx1cpS/nRovqp3gPnau4ch86R uozZzG1a3MaAG1HjAGhyCCnne06VZYeIa+E2CSgFtjJbjg9eS9oGCqo2nBbMd/rT/c8G DyKzYZPf4AWB+3QUrgjO2kbYevLb6ROLvjTGi+bqUo4W9MtlS1R4TQUolhu+8Qm7vL/d XubGSM6vMQ9KQ0+U7gsXwVnOY41Tj8SAK1Bo7qmoYOSygZHNpRPg6GoUNodZ/X39Lymz JXRQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1764876165; x=1765480965; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=qRUfxBLVaD2BUiIJM9PrjNYt0vrl8q05tvZ2QQvcI7c=; b=MS0OP9Ht5F8QxA/W6iD+qVWgR4ZLVyCkVSzWT5zbZv40g6LedElw4ekhVyCuU5pYOV vuIXuG61ezw8TAE7fVFZaRAHeiWy3mH2OaMSCtjV6A9xVH0yNHz5mJc2c5AexzUJhW4h UX+K+X8jnZLJnkI/FkfrLN4kfpnkFpQi91E3ETx2kQpZqdzFP8foD+t/YDWFAmeap6oW ywDMV5vbBz+nLSWZUrnihS3hPeyhoGaKrecIAZuAdOuPcYUIHvwjjcedixD7cxXzfxsi gN/nrgtTW1fas7Uv4NTES2eix69DV+LegfxE/w9n88N5D2dqnijINQDlybZqNEfqCJMJ Gejw== X-Forwarded-Encrypted: i=1; AJvYcCVWARQb+NGvFUIDXNqHDmcOb5yIqB+4gayaC+arGW1m8I0Fqlh/Vc0dMTe25f2Qsl2I7k6ZQezO6QPOa8dr@vger.kernel.org X-Gm-Message-State: AOJu0YxtG8Da6kNRZT6qDG5PO8XBk8+FEjgBYKE1Vdr4Vk+VvBNpAXS/ SgXkjT7rMsfWXQSUH60MJKjAjVRweryhIldv7f/VSaAIhPsrck31yqYMmRBAPgj+Bia66Vm3QGW bAcPSdRXVBDJCPeN/pjvN7F6It0qhRdM= X-Gm-Gg: ASbGncsZTco+BX1S4snz2K5pCbS14VdP+bJyyGl33q7AiLGvNE4dMHcECB3oegMUBzH 6BFda4tIfkD5VhgzN4rG4rrvAfywrzAjOonHhhJ4xwBImuqiWj7z2asuhfe2KzNaTq3s/qPXWhK vpoTC4qIBXEJO/Gh89tKMeIMciHNuphs1A9XKMM1ZCmW3RHfK6pKQDfBBOwdPmKRguO4ExG16g0 5XGfEGOBssSZ4i90ijzZSpySVYI6pf1pUIIZZCnbhpqVQPhPP7vpmUTPhnGbgOXtOpiZfhMhMqK ROZX X-Google-Smtp-Source: AGHT+IFPMbh323+JAvtLwyERQYu3+2/PbZpT5HWq8js0GyV+H7LZEZt4a7q3yAiXV9JX2fETPaDniXWoVq/0a+fgvQI= X-Received: by 2002:a05:622a:2c2:b0:4ee:4a3a:bd0f with SMTP id d75a77b69052e-4f0176955f0mr94281971cf.69.1764876165408; Thu, 04 Dec 2025 11:22:45 -0800 (PST) Precedence: bulk X-Mailing-List: linux-fsdevel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <20251203003526.2889477-1-joannelkoong@gmail.com> <20251203003526.2889477-12-joannelkoong@gmail.com> In-Reply-To: From: Joanne Koong Date: Thu, 4 Dec 2025 11:22:34 -0800 X-Gm-Features: AWmQ_ble_W30mrAwwzaBfd_EZgJctBffhWFoX0ouusrSPuhfTQBV9AthDu10r9Y Message-ID: Subject: Re: [PATCH v1 11/30] io_uring/kbuf: return buffer id in buffer selection To: Caleb Sander Mateos Cc: miklos@szeredi.hu, axboe@kernel.dk, bschubert@ddn.com, asml.silence@gmail.com, io-uring@vger.kernel.org, xiaobing.li@samsung.com, linux-fsdevel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, Dec 3, 2025 at 1:54=E2=80=AFPM Caleb Sander Mateos wrote: > > On Tue, Dec 2, 2025 at 4:36=E2=80=AFPM Joanne Koong wrote: > > > > Return the id of the selected buffer in io_buffer_select(). This is > > needed for kernel-managed buffer rings to later recycle the selected > > buffer. > > > > Signed-off-by: Joanne Koong > > --- > > include/linux/io_uring/cmd.h | 2 +- > > include/linux/io_uring_types.h | 2 ++ > > io_uring/kbuf.c | 7 +++++-- > > 3 files changed, 8 insertions(+), 3 deletions(-) > > > > diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_ty= pes.h > > index e1a75cfe57d9..dcc95e73f12f 100644 > > --- a/include/linux/io_uring_types.h > > +++ b/include/linux/io_uring_types.h > > @@ -109,6 +109,8 @@ struct io_br_sel { > > void *kaddr; > > }; > > ssize_t val; > > + /* id of the selected buffer */ > > + unsigned buf_id; > > Looks like this could be unioned with val? I think val's size can be > reduced to an int since only int values are assigned to it. > I'm not sure I see the advantage of this. imo I think it makes the interface a bit more confusing, as val also represents the error code and is logically its own separate entity from buf_id. I don't see the struct io_br_sel being stored anywhere where it seems important to save a few bytes, but maybe I'm missing something here. > > }; > > > > > > diff --git a/io_uring/kbuf.c b/io_uring/kbuf.c > > index 8a94de6e530f..3ecb6494adea 100644 > > --- a/io_uring/kbuf.c > > +++ b/io_uring/kbuf.c > > @@ -239,6 +239,7 @@ static struct io_br_sel io_ring_buffer_select(struc= t io_kiocb *req, size_t *len, > > req->flags |=3D REQ_F_BUFFER_RING | REQ_F_BUFFERS_COMMIT; > > req->buf_index =3D buf->bid; > > sel.buf_list =3D bl; > > + sel.buf_id =3D buf->bid; > > This is userspace mapped, so probably should be using READ_ONCE() and > reusing the value between req->buf_index and buf->bid? Looks like an > existing bug that the reads of buf->bid and buf->addr aren't using > READ_ONCE(). Agreed. I think that existing bug should be its own patch. Are you planning to submit that fix? since you found that bug, I think you should get the authorship for it, but if it's annoying for whatever reason and you prefer not to, I can roll that up into this patchset. EIther way, I'll rebase v2 on top of that and change this line to "sel.buf_id =3D req->buf_index". > > > if (bl->flags & IOBL_KERNEL_MANAGED) > > sel.kaddr =3D (void *)buf->addr; > > else > > @@ -262,10 +263,12 @@ struct io_br_sel io_buffer_select(struct io_kiocb= *req, size_t *len, > > > > bl =3D io_buffer_get_list(ctx, buf_group); > > if (likely(bl)) { > > - if (bl->flags & IOBL_BUF_RING) > > + if (bl->flags & IOBL_BUF_RING) { > > sel =3D io_ring_buffer_select(req, len, bl, iss= ue_flags); > > - else > > + } else { > > sel.addr =3D io_provided_buffer_select(req, len= , bl); > > + sel.buf_id =3D req->buf_index; > > Could this cover both IOBL_BUF_RING and !IOBL_BUF_RING cases to avoid > the additional logic in io_ring_buffer_select()? Ah the next patch (patch 12/30: io_uring/kbuf: export io_ring_buffer_select() [1]) exports io_ring_buffer_select() to be directly called, so that was my motivation for having that sel.buf_id assignment logic in io_ring_buffer_select(). I should have swapped the ordering of those 2 patches to make that more clear, I'll do that for v2. [1] https://lore.kernel.org/linux-fsdevel/20251203003526.2889477-13-joannel= koong@gmail.com/ Thanks, Joanne > > Best, > Caleb > > > + } > > } > > io_ring_submit_unlock(req->ctx, issue_flags); > > return sel; > > -- > > 2.47.3 > >