| Message ID | 9b2e791d-aeba-a2c7-1877-797ff5c680a3@kernel.dk (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [GIT,PULL] Final io_uring updates for 6.4-rc1 | expand |
On Sun, May 7, 2023 at 5:00 AM Jens Axboe <axboe@kernel.dk> wrote: > > Note that this will throw a merge conflict in the ublk_drv code, due > to this branch still being based off the original for-6.4/io_uring > branch. Resolution is pretty straight forward, I'm including it below > for reference. Mine is somewhat different - I think the "ub_cmd" argument to __ublk_ch_uring_cmd() should also have been made a 'const' pointer. And then using an actual initializer allows us to do the same thing for the copy in ublk_ch_uring_cmd() (and also makes it clear that it initializes the whole struct - which it did, but still...) So my conflict resolution looks a bit more complicated, but I think it's the RightThing(tm) to do, and is consistent with the constification in commit fd9b8547bc5c ("io_uring: Pass whole sqe to commands"). Linus
The pull request you sent on Sun, 7 May 2023 06:00:48 -0600:
> git://git.kernel.dk/linux.git tags/for-6.4/io_uring-2023-05-07
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/03e5cb7b50feb687508946a702febaba24c77f0b
Thank you!
Hi, On Sun, May 07, 2023 at 06:00:48AM -0600, Jens Axboe wrote: > Hi Linus, > > Nothing major in here, just two different parts: > > - Small series from Breno that enables passing the full SQE down > for ->uring_cmd(). This is a prerequisite for enabling full network > socket operations. Queued up a bit late because of some stylistic > concerns that got resolved, would be nice to have this in 6.4-rc1 > so the dependent work will be easier to handle for 6.5. > > - Fix for the huge page coalescing, which was a regression introduced > in the 6.3 kernel release (Tobias). > > Note that this will throw a merge conflict in the ublk_drv code, due > to this branch still being based off the original for-6.4/io_uring > branch. Resolution is pretty straight forward, I'm including it below > for reference. > > Please pull! > > > The following changes since commit 3c85cc43c8e7855d202da184baf00c7b8eeacf71: > > Revert "io_uring/rsrc: disallow multi-source reg buffers" (2023-04-20 06:51:48 -0600) > > are available in the Git repository at: > > git://git.kernel.dk/linux.git tags/for-6.4/io_uring-2023-05-07 > > for you to fetch changes up to d2b7fa6174bc4260e496cbf84375c73636914641: > > io_uring: Remove unnecessary BUILD_BUG_ON (2023-05-04 08:19:05 -0600) > > ---------------------------------------------------------------- > for-6.4/io_uring-2023-05-07 > > ---------------------------------------------------------------- > Breno Leitao (3): > io_uring: Create a helper to return the SQE size > io_uring: Pass whole sqe to commands This commit causes broken builds when IO_URING=n and NVME_CORE=y, as io_uring_sqe_cmd(), called in drivers/nvme/host/ioctl.c, ends up being undefined. This was also reported [1] by 0-day bot on your branch yesterday, but it's worse now that Linus merged the pull request. Not sure what the better fix would be. Move io_uring_sqe_cmd() outside of the "#if defined(CONFIG_IO_URING)" block? ChenYu [1] https://lore.kernel.org/all/202305080039.r7cguaXB-lkp@intel.com/ > io_uring: Remove unnecessary BUILD_BUG_ON > > Tobias Holl (1): > io_uring/rsrc: check for nonconsecutive pages > > drivers/block/ublk_drv.c | 26 +++++++++++++------------- > drivers/nvme/host/ioctl.c | 2 +- > include/linux/io_uring.h | 7 ++++++- > io_uring/io_uring.h | 10 ++++++++++ > io_uring/opdef.c | 2 +- > io_uring/rsrc.c | 7 ++++++- > io_uring/uring_cmd.c | 12 +++--------- > io_uring/uring_cmd.h | 8 -------- > 8 files changed, 40 insertions(+), 34 deletions(-) > > Merge resolution: > > commit 775e045e380626ce769d95badc79ea08edc1b15d > Merge: efd141da30bb d2b7fa6174bc > Author: Jens Axboe <axboe@kernel.dk> > Date: Thu May 4 08:22:24 2023 -0600 > > Merge branch 'for-6.4/io_uring' into test > > * for-6.4/io_uring: > io_uring: Remove unnecessary BUILD_BUG_ON > io_uring: Pass whole sqe to commands > io_uring: Create a helper to return the SQE size > io_uring/rsrc: check for nonconsecutive pages > > Signed-off-by: Jens Axboe <axboe@kernel.dk> > > diff --cc drivers/block/ublk_drv.c > index 72a5cde9a5af,42f4d7ca962e..0af6a41f92b2 > --- a/drivers/block/ublk_drv.c > +++ b/drivers/block/ublk_drv.c > @@@ -1397,23 -1362,6 +1397,23 @@@ static int __ublk_ch_uring_cmd(struct i > return -EIOCBQUEUED; > } > > +static int ublk_ch_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags) > +{ > - struct ublksrv_io_cmd *ub_src = (struct ublksrv_io_cmd *) cmd->cmd; > ++ const struct ublksrv_io_cmd *ub_src = io_uring_sqe_cmd(cmd->sqe); > + struct ublksrv_io_cmd ub_cmd; > + > + /* > + * Not necessary for async retry, but let's keep it simple and always > + * copy the values to avoid any potential reuse. > + */ > + ub_cmd.q_id = READ_ONCE(ub_src->q_id); > + ub_cmd.tag = READ_ONCE(ub_src->tag); > + ub_cmd.result = READ_ONCE(ub_src->result); > + ub_cmd.addr = READ_ONCE(ub_src->addr); > + > + return __ublk_ch_uring_cmd(cmd, issue_flags, &ub_cmd); > +} > + > static const struct file_operations ublk_ch_fops = { > .owner = THIS_MODULE, > .open = ublk_ch_open, > @@@ -2240,9 -2171,8 +2240,9 @@@ exit > static int ublk_ctrl_uring_cmd(struct io_uring_cmd *cmd, > unsigned int issue_flags) > { > - struct ublksrv_ctrl_cmd *header = (struct ublksrv_ctrl_cmd *)cmd->cmd; > + const struct ublksrv_ctrl_cmd *header = io_uring_sqe_cmd(cmd->sqe); > struct ublk_device *ub = NULL; > + u32 cmd_op = cmd->cmd_op; > int ret = -EINVAL; > > if (issue_flags & IO_URING_F_NONBLOCK) > > -- > Jens Axboe > >
On 5/7/23 10:59?AM, Linus Torvalds wrote: > On Sun, May 7, 2023 at 5:00?AM Jens Axboe <axboe@kernel.dk> wrote: >> >> Note that this will throw a merge conflict in the ublk_drv code, due >> to this branch still being based off the original for-6.4/io_uring >> branch. Resolution is pretty straight forward, I'm including it below >> for reference. > > Mine is somewhat different - I think the "ub_cmd" argument to > __ublk_ch_uring_cmd() should also have been made a 'const' pointer. > > And then using an actual initializer allows us to do the same thing > for the copy in ublk_ch_uring_cmd() (and also makes it clear that it > initializes the whole struct - which it did, but still...) > > So my conflict resolution looks a bit more complicated, but I think > it's the RightThing(tm) to do, and is consistent with the > constification in commit fd9b8547bc5c ("io_uring: Pass whole sqe to > commands"). Your resolution is obviously fine, and I did ponder pushing the constification further. Thanks!
On 5/7/23 9:18?PM, Chen-Yu Tsai wrote: > Hi, > > On Sun, May 07, 2023 at 06:00:48AM -0600, Jens Axboe wrote: >> Hi Linus, >> >> Nothing major in here, just two different parts: >> >> - Small series from Breno that enables passing the full SQE down >> for ->uring_cmd(). This is a prerequisite for enabling full network >> socket operations. Queued up a bit late because of some stylistic >> concerns that got resolved, would be nice to have this in 6.4-rc1 >> so the dependent work will be easier to handle for 6.5. >> >> - Fix for the huge page coalescing, which was a regression introduced >> in the 6.3 kernel release (Tobias). >> >> Note that this will throw a merge conflict in the ublk_drv code, due >> to this branch still being based off the original for-6.4/io_uring >> branch. Resolution is pretty straight forward, I'm including it below >> for reference. >> >> Please pull! >> >> >> The following changes since commit 3c85cc43c8e7855d202da184baf00c7b8eeacf71: >> >> Revert "io_uring/rsrc: disallow multi-source reg buffers" (2023-04-20 06:51:48 -0600) >> >> are available in the Git repository at: >> >> git://git.kernel.dk/linux.git tags/for-6.4/io_uring-2023-05-07 >> >> for you to fetch changes up to d2b7fa6174bc4260e496cbf84375c73636914641: >> >> io_uring: Remove unnecessary BUILD_BUG_ON (2023-05-04 08:19:05 -0600) >> >> ---------------------------------------------------------------- >> for-6.4/io_uring-2023-05-07 >> >> ---------------------------------------------------------------- >> Breno Leitao (3): >> io_uring: Create a helper to return the SQE size >> io_uring: Pass whole sqe to commands > > This commit causes broken builds when IO_URING=n and NVME_CORE=y, as > io_uring_sqe_cmd(), called in drivers/nvme/host/ioctl.c, ends up being > undefined. This was also reported [1] by 0-day bot on your branch > yesterday, but it's worse now that Linus merged the pull request. > > Not sure what the better fix would be. Move io_uring_sqe_cmd() outside > of the "#if defined(CONFIG_IO_URING)" block? Queued up a patch for this: https://git.kernel.dk/cgit/linux/commit/?h=io_uring-6.4&id=5d371b2f2b0d1a047582563ee36af8ffb5022847
On Tue, May 9, 2023 at 2:42 AM Jens Axboe <axboe@kernel.dk> wrote: > > On 5/7/23 9:18?PM, Chen-Yu Tsai wrote: > > Hi, > > > > On Sun, May 07, 2023 at 06:00:48AM -0600, Jens Axboe wrote: > >> Hi Linus, > >> > >> Nothing major in here, just two different parts: > >> > >> - Small series from Breno that enables passing the full SQE down > >> for ->uring_cmd(). This is a prerequisite for enabling full network > >> socket operations. Queued up a bit late because of some stylistic > >> concerns that got resolved, would be nice to have this in 6.4-rc1 > >> so the dependent work will be easier to handle for 6.5. > >> > >> - Fix for the huge page coalescing, which was a regression introduced > >> in the 6.3 kernel release (Tobias). > >> > >> Note that this will throw a merge conflict in the ublk_drv code, due > >> to this branch still being based off the original for-6.4/io_uring > >> branch. Resolution is pretty straight forward, I'm including it below > >> for reference. > >> > >> Please pull! > >> > >> > >> The following changes since commit 3c85cc43c8e7855d202da184baf00c7b8eeacf71: > >> > >> Revert "io_uring/rsrc: disallow multi-source reg buffers" (2023-04-20 06:51:48 -0600) > >> > >> are available in the Git repository at: > >> > >> git://git.kernel.dk/linux.git tags/for-6.4/io_uring-2023-05-07 > >> > >> for you to fetch changes up to d2b7fa6174bc4260e496cbf84375c73636914641: > >> > >> io_uring: Remove unnecessary BUILD_BUG_ON (2023-05-04 08:19:05 -0600) > >> > >> ---------------------------------------------------------------- > >> for-6.4/io_uring-2023-05-07 > >> > >> ---------------------------------------------------------------- > >> Breno Leitao (3): > >> io_uring: Create a helper to return the SQE size > >> io_uring: Pass whole sqe to commands > > > > This commit causes broken builds when IO_URING=n and NVME_CORE=y, as > > io_uring_sqe_cmd(), called in drivers/nvme/host/ioctl.c, ends up being > > undefined. This was also reported [1] by 0-day bot on your branch > > yesterday, but it's worse now that Linus merged the pull request. > > > > Not sure what the better fix would be. Move io_uring_sqe_cmd() outside > > of the "#if defined(CONFIG_IO_URING)" block? > > Queued up a patch for this: > > https://git.kernel.dk/cgit/linux/commit/?h=io_uring-6.4&id=5d371b2f2b0d1a047582563ee36af8ffb5022847 Thanks! Looks like the Reported-by line for the test bot is missing a right angle bracket? Also, consider it Tested-by: Chen-Yu Tsai <wenst@chromium.org>
On 5/8/23 8:35?PM, Chen-Yu Tsai wrote: > On Tue, May 9, 2023 at 2:42?AM Jens Axboe <axboe@kernel.dk> wrote: >> >> On 5/7/23 9:18?PM, Chen-Yu Tsai wrote: >>> Hi, >>> >>> On Sun, May 07, 2023 at 06:00:48AM -0600, Jens Axboe wrote: >>>> Hi Linus, >>>> >>>> Nothing major in here, just two different parts: >>>> >>>> - Small series from Breno that enables passing the full SQE down >>>> for ->uring_cmd(). This is a prerequisite for enabling full network >>>> socket operations. Queued up a bit late because of some stylistic >>>> concerns that got resolved, would be nice to have this in 6.4-rc1 >>>> so the dependent work will be easier to handle for 6.5. >>>> >>>> - Fix for the huge page coalescing, which was a regression introduced >>>> in the 6.3 kernel release (Tobias). >>>> >>>> Note that this will throw a merge conflict in the ublk_drv code, due >>>> to this branch still being based off the original for-6.4/io_uring >>>> branch. Resolution is pretty straight forward, I'm including it below >>>> for reference. >>>> >>>> Please pull! >>>> >>>> >>>> The following changes since commit 3c85cc43c8e7855d202da184baf00c7b8eeacf71: >>>> >>>> Revert "io_uring/rsrc: disallow multi-source reg buffers" (2023-04-20 06:51:48 -0600) >>>> >>>> are available in the Git repository at: >>>> >>>> git://git.kernel.dk/linux.git tags/for-6.4/io_uring-2023-05-07 >>>> >>>> for you to fetch changes up to d2b7fa6174bc4260e496cbf84375c73636914641: >>>> >>>> io_uring: Remove unnecessary BUILD_BUG_ON (2023-05-04 08:19:05 -0600) >>>> >>>> ---------------------------------------------------------------- >>>> for-6.4/io_uring-2023-05-07 >>>> >>>> ---------------------------------------------------------------- >>>> Breno Leitao (3): >>>> io_uring: Create a helper to return the SQE size >>>> io_uring: Pass whole sqe to commands >>> >>> This commit causes broken builds when IO_URING=n and NVME_CORE=y, as >>> io_uring_sqe_cmd(), called in drivers/nvme/host/ioctl.c, ends up being >>> undefined. This was also reported [1] by 0-day bot on your branch >>> yesterday, but it's worse now that Linus merged the pull request. >>> >>> Not sure what the better fix would be. Move io_uring_sqe_cmd() outside >>> of the "#if defined(CONFIG_IO_URING)" block? >> >> Queued up a patch for this: >> >> https://git.kernel.dk/cgit/linux/commit/?h=io_uring-6.4&id=5d371b2f2b0d1a047582563ee36af8ffb5022847 > > Thanks! Looks like the Reported-by line for the test bot is missing a right > angle bracket? > > Also, consider it > > Tested-by: Chen-Yu Tsai <wenst@chromium.org> Oops yes, thanks for noticing. I'll correct that and add your tested-by.
On Tue, May 9, 2023 at 6:59 AM Jens Axboe <axboe@kernel.dk> wrote: > > Oops yes, thanks for noticing. I'll correct that and add your tested-by. .. this build problem isn't caught by Guenter's build testing, but can we please make sure that it's fixed in my tree by -rc2? It would be lovely to have a release that doesn't end up having silly build problems pending for too long. Even if they might be unusual configs. Linus
On 5/9/23 10:52?AM, Linus Torvalds wrote: > On Tue, May 9, 2023 at 6:59?AM Jens Axboe <axboe@kernel.dk> wrote: >> >> Oops yes, thanks for noticing. I'll correct that and add your tested-by. > > .. this build problem isn't caught by Guenter's build testing, but can > we please make sure that it's fixed in my tree by -rc2? > > It would be lovely to have a release that doesn't end up having silly > build problems pending for too long. Even if they might be unusual > configs. It's in my tree, it'll go your way before -rc2 for sure.
diff --cc drivers/block/ublk_drv.c index 72a5cde9a5af,42f4d7ca962e..0af6a41f92b2 --- a/drivers/block/ublk_drv.c
Hi Linus, Nothing major in here, just two different parts: - Small series from Breno that enables passing the full SQE down for ->uring_cmd(). This is a prerequisite for enabling full network socket operations. Queued up a bit late because of some stylistic concerns that got resolved, would be nice to have this in 6.4-rc1 so the dependent work will be easier to handle for 6.5. - Fix for the huge page coalescing, which was a regression introduced in the 6.3 kernel release (Tobias). Note that this will throw a merge conflict in the ublk_drv code, due to this branch still being based off the original for-6.4/io_uring branch. Resolution is pretty straight forward, I'm including it below for reference. Please pull! The following changes since commit 3c85cc43c8e7855d202da184baf00c7b8eeacf71: Revert "io_uring/rsrc: disallow multi-source reg buffers" (2023-04-20 06:51:48 -0600) are available in the Git repository at: git://git.kernel.dk/linux.git tags/for-6.4/io_uring-2023-05-07 for you to fetch changes up to d2b7fa6174bc4260e496cbf84375c73636914641: io_uring: Remove unnecessary BUILD_BUG_ON (2023-05-04 08:19:05 -0600) ---------------------------------------------------------------- for-6.4/io_uring-2023-05-07 ---------------------------------------------------------------- Breno Leitao (3): io_uring: Create a helper to return the SQE size io_uring: Pass whole sqe to commands io_uring: Remove unnecessary BUILD_BUG_ON Tobias Holl (1): io_uring/rsrc: check for nonconsecutive pages drivers/block/ublk_drv.c | 26 +++++++++++++------------- drivers/nvme/host/ioctl.c | 2 +- include/linux/io_uring.h | 7 ++++++- io_uring/io_uring.h | 10 ++++++++++ io_uring/opdef.c | 2 +- io_uring/rsrc.c | 7 ++++++- io_uring/uring_cmd.c | 12 +++--------- io_uring/uring_cmd.h | 8 -------- 8 files changed, 40 insertions(+), 34 deletions(-) Merge resolution: commit 775e045e380626ce769d95badc79ea08edc1b15d Merge: efd141da30bb d2b7fa6174bc Author: Jens Axboe <axboe@kernel.dk> Date: Thu May 4 08:22:24 2023 -0600 Merge branch 'for-6.4/io_uring' into test * for-6.4/io_uring: io_uring: Remove unnecessary BUILD_BUG_ON io_uring: Pass whole sqe to commands io_uring: Create a helper to return the SQE size io_uring/rsrc: check for nonconsecutive pages Signed-off-by: Jens Axboe <axboe@kernel.dk> +++ b/drivers/block/ublk_drv.c @@@ -1397,23 -1362,6 +1397,23 @@@ static int __ublk_ch_uring_cmd(struct i return -EIOCBQUEUED; } +static int ublk_ch_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags) +{ - struct ublksrv_io_cmd *ub_src = (struct ublksrv_io_cmd *) cmd->cmd; ++ const struct ublksrv_io_cmd *ub_src = io_uring_sqe_cmd(cmd->sqe); + struct ublksrv_io_cmd ub_cmd; + + /* + * Not necessary for async retry, but let's keep it simple and always + * copy the values to avoid any potential reuse. + */ + ub_cmd.q_id = READ_ONCE(ub_src->q_id); + ub_cmd.tag = READ_ONCE(ub_src->tag); + ub_cmd.result = READ_ONCE(ub_src->result); + ub_cmd.addr = READ_ONCE(ub_src->addr); + + return __ublk_ch_uring_cmd(cmd, issue_flags, &ub_cmd); +} + static const struct file_operations ublk_ch_fops = { .owner = THIS_MODULE, .open = ublk_ch_open, @@@ -2240,9 -2171,8 +2240,9 @@@ exit static int ublk_ctrl_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags) { - struct ublksrv_ctrl_cmd *header = (struct ublksrv_ctrl_cmd *)cmd->cmd; + const struct ublksrv_ctrl_cmd *header = io_uring_sqe_cmd(cmd->sqe); struct ublk_device *ub = NULL; + u32 cmd_op = cmd->cmd_op; int ret = -EINVAL; if (issue_flags & IO_URING_F_NONBLOCK)