| Message ID | 20220215180328.2320199-1-shr@fb.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | io-uring: Make statx api stable | expand |
On 2/15/22 11:03 AM, Stefan Roesch wrote: > One of the key architectual tenets of io-uring is to keep the > parameters for io-uring stable. After the call has been submitted, > its value can be changed. Unfortunaltely this is not the case for > the current statx implementation. > > Patches: > Patch 1: fs: replace const char* parameter in vfs_statx and do_statx with > struct filename > Create filename object outside of do_statx and vfs_statx, so io-uring > can create the filename object during the prepare phase > > Patch 2: io-uring: Copy path name during prepare stage for statx > Create and store filename object during prepare phase > > > There is also a patch for the liburing libray to add a new test case. This > patch makes sure that the api is stable. > "liburing: add test for stable statx api" > > The patch has been tested with the liburing test suite and fstests. Al, are you happy with this version?
On 2/18/22 9:15 AM, Jens Axboe wrote: > On 2/15/22 11:03 AM, Stefan Roesch wrote: >> One of the key architectual tenets of io-uring is to keep the >> parameters for io-uring stable. After the call has been submitted, >> its value can be changed. Unfortunaltely this is not the case for >> the current statx implementation. >> >> Patches: >> Patch 1: fs: replace const char* parameter in vfs_statx and do_statx with >> struct filename >> Create filename object outside of do_statx and vfs_statx, so io-uring >> can create the filename object during the prepare phase >> >> Patch 2: io-uring: Copy path name during prepare stage for statx >> Create and store filename object during prepare phase >> >> >> There is also a patch for the liburing libray to add a new test case. This >> patch makes sure that the api is stable. >> "liburing: add test for stable statx api" >> >> The patch has been tested with the liburing test suite and fstests. > > Al, are you happy with this version? I have staged this one for 5.18, it's in for-5.18/io_uring-statx. It will be sent separately from the general io_uring fixes/updates.
On Tue, 15 Feb 2022 10:03:26 -0800, Stefan Roesch wrote: > One of the key architectual tenets of io-uring is to keep the > parameters for io-uring stable. After the call has been submitted, > its value can be changed. Unfortunaltely this is not the case for > the current statx implementation. > > Patches: > Patch 1: fs: replace const char* parameter in vfs_statx and do_statx with > struct filename > Create filename object outside of do_statx and vfs_statx, so io-uring > can create the filename object during the prepare phase > > [...] Applied, thanks! [1/2] fs: replace const char* parameter in vfs_statx and do_statx with struct filename commit: 30512d54fae354a2359a740b75a1451b68aa3807 [2/2] io-uring: Copy path name during prepare stage for statx commit: 1e0561928e3ab5018615403a2a1293e1e44ee03e Best regards,
Hi, On 22.02.2022 19:45, Jens Axboe wrote: > On Tue, 15 Feb 2022 10:03:26 -0800, Stefan Roesch wrote: >> One of the key architectual tenets of io-uring is to keep the >> parameters for io-uring stable. After the call has been submitted, >> its value can be changed. Unfortunaltely this is not the case for >> the current statx implementation. >> >> Patches: >> Patch 1: fs: replace const char* parameter in vfs_statx and do_statx with >> struct filename >> Create filename object outside of do_statx and vfs_statx, so io-uring >> can create the filename object during the prepare phase >> >> [...] > Applied, thanks! > > [1/2] fs: replace const char* parameter in vfs_statx and do_statx with struct filename > commit: 30512d54fae354a2359a740b75a1451b68aa3807 > [2/2] io-uring: Copy path name during prepare stage for statx > commit: 1e0561928e3ab5018615403a2a1293e1e44ee03e Those 2 commits landed in todays Linux next-20220223. They affect userspace in a way that breaks systemd opration: ... Freeing unused kernel image (initmem) memory: 1024K Run /sbin/init as init process systemd[1]: System time before build time, advancing clock. systemd[1]: Cannot be run in a chroot() environment. systemd[1]: Freezing execution. Reverting them on top of next-20220223 fixes the boot issue. Btw, those patches are not bisectable. The code at 30512d54fae354a2359a740b75a1451b68aa3807 doesn't compile. Best regards
On 2/24/22 5:47 AM, Marek Szyprowski wrote: > Hi, > > On 22.02.2022 19:45, Jens Axboe wrote: >> On Tue, 15 Feb 2022 10:03:26 -0800, Stefan Roesch wrote: >>> One of the key architectual tenets of io-uring is to keep the >>> parameters for io-uring stable. After the call has been submitted, >>> its value can be changed. Unfortunaltely this is not the case for >>> the current statx implementation. >>> >>> Patches: >>> Patch 1: fs: replace const char* parameter in vfs_statx and do_statx with >>> struct filename >>> Create filename object outside of do_statx and vfs_statx, so io-uring >>> can create the filename object during the prepare phase >>> >>> [...] >> Applied, thanks! >> >> [1/2] fs: replace const char* parameter in vfs_statx and do_statx with struct filename >> commit: 30512d54fae354a2359a740b75a1451b68aa3807 >> [2/2] io-uring: Copy path name during prepare stage for statx >> commit: 1e0561928e3ab5018615403a2a1293e1e44ee03e > > Those 2 commits landed in todays Linux next-20220223. They affect > userspace in a way that breaks systemd opration: > > ... > > Freeing unused kernel image (initmem) memory: 1024K > Run /sbin/init as init process > systemd[1]: System time before build time, advancing clock. > systemd[1]: Cannot be run in a chroot() environment. > systemd[1]: Freezing execution. > > Reverting them on top of next-20220223 fixes the boot issue. Btw, those > patches are not bisectable. The code at > 30512d54fae354a2359a740b75a1451b68aa3807 doesn't compile. Thanks, I'll drop them from for-next until we figure out what that is.
On Thu, Feb 24, 2022 at 07:09:35AM -0700, Jens Axboe wrote: > > Freeing unused kernel image (initmem) memory: 1024K > > Run /sbin/init as init process > > systemd[1]: System time before build time, advancing clock. > > systemd[1]: Cannot be run in a chroot() environment. > > systemd[1]: Freezing execution. > > > > Reverting them on top of next-20220223 fixes the boot issue. Btw, those > > patches are not bisectable. The code at > > 30512d54fae354a2359a740b75a1451b68aa3807 doesn't compile. > > Thanks, I'll drop them from for-next until we figure out what that is. FYI, this breaks the boot on bare-metal as well. Loading, please wait... Starting version 245.4-4ubuntu3.15 Running in chroot, ignoring request. Running in chroot, ignoring request. Running in chroot, ignoring request. Running in chroot, ignoring request. Running in chroot, ignoring request. /scripts/init-top/console_setup: line 90: can't create /dev/tty1: No such device or address /scripts/init-top/console_setup: line 126: can't open /dev/tty1: No such device or address Begin: Loading essential drivers ... /init: line 155: modprobe: Permission denied /init: line 155: modprobe: Permission denied /init: line 155: modprobe: Permission denied /init: line 155: modprobe: Permission denied /init: line 155: modprobe: Permission denied /init: line 155: modprobe: Permission denied /init: line 155: modprobe: Permission denied /init: line 155: modprobe: Permission denied
On Thu, Feb 24, 2022 at 08:27:24PM -0500, Qian Cai wrote: > On Thu, Feb 24, 2022 at 07:09:35AM -0700, Jens Axboe wrote: > > > Freeing unused kernel image (initmem) memory: 1024K > > > Run /sbin/init as init process > > > systemd[1]: System time before build time, advancing clock. > > > systemd[1]: Cannot be run in a chroot() environment. > > > systemd[1]: Freezing execution. > > > > > > Reverting them on top of next-20220223 fixes the boot issue. Btw, those > > > patches are not bisectable. The code at > > > 30512d54fae354a2359a740b75a1451b68aa3807 doesn't compile. > > > > Thanks, I'll drop them from for-next until we figure out what that is. > > FYI, this breaks the boot on bare-metal as well. It also broke my boot on a simple debian testing KVM guest as well. Reverting those two commits fixes my boot. Jens, any chance we can include / ask for a bit more run time testing? What sort of tests get run ? Luis
One of the key architectual tenets of io-uring is to keep the parameters for io-uring stable. After the call has been submitted, its value can be changed. Unfortunaltely this is not the case for the current statx implementation. Patches: Patch 1: fs: replace const char* parameter in vfs_statx and do_statx with struct filename Create filename object outside of do_statx and vfs_statx, so io-uring can create the filename object during the prepare phase Patch 2: io-uring: Copy path name during prepare stage for statx Create and store filename object during prepare phase There is also a patch for the liburing libray to add a new test case. This patch makes sure that the api is stable. "liburing: add test for stable statx api" The patch has been tested with the liburing test suite and fstests. Changes: V2: don't check name in vfs_fstatat V3: don't check name in statx syscall Stefan Roesch (2): fs: replace const char* parameter in vfs_statx and do_statx with struct filename io-uring: Copy path name during prepare stage for statx fs/internal.h | 4 +++- fs/io_uring.c | 22 ++++++++++++++++++++-- fs/stat.c | 47 ++++++++++++++++++++++++++++++++++------------- 3 files changed, 57 insertions(+), 16 deletions(-) base-commit: 705d84a366cfccda1e7aec1113a5399cd2ffee7d