| Message ID | 20220531184125.2665210-1-usama.arif@bytedance.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | io_uring: add opcodes for current working directory | expand |
On 5/31/22 12:41 PM, Usama Arif wrote: > This provides consistency between io_uring and the respective I/O syscall > and avoids having the user of liburing specify the cwd in sqe when working > with current working directory, for e.g. the user can directly call with > IORING_OP_RENAME instead of IORING_OP_RENAMEAT and providing AT_FDCWD in > sqe->fd and sqe->len, similar to syscall interface. > > This is done for rename, unlink, mkdir, symlink and link in this > patch-series. > > The tests for these opcodes in liburing are present at > https://github.com/uarif1/liburing/tree/cwd_opcodes. If the patches are > acceptable, I am happy to create a PR in above for the tests. Can't we just provide prep helpers for them in liburing?
On 31/05/2022 19:58, Jens Axboe wrote: > On 5/31/22 12:41 PM, Usama Arif wrote: >> This provides consistency between io_uring and the respective I/O syscall >> and avoids having the user of liburing specify the cwd in sqe when working >> with current working directory, for e.g. the user can directly call with >> IORING_OP_RENAME instead of IORING_OP_RENAMEAT and providing AT_FDCWD in >> sqe->fd and sqe->len, similar to syscall interface. >> >> This is done for rename, unlink, mkdir, symlink and link in this >> patch-series. >> >> The tests for these opcodes in liburing are present at >> https://github.com/uarif1/liburing/tree/cwd_opcodes. If the patches are >> acceptable, I am happy to create a PR in above for the tests. > > Can't we just provide prep helpers for them in liburing? > We could add a io_uring_prep_unlink with IORING_OP_UNLINKAT and AT_FDCWD in liburing. But i guess adding in kernel adds a more consistent interface? and allows to make calls bypassing liburing (although i guess people probably don't bypass liburing that much :)) Making the changes in both kernel and liburing provides more of a standard interface in my opinion so maybe it looks better. But happy to just create a PR in liburing only with prep helpers as you suggested if you think that is better? Thanks!
On 5/31/22 1:18 PM, Usama Arif wrote: > > > On 31/05/2022 19:58, Jens Axboe wrote: >> On 5/31/22 12:41 PM, Usama Arif wrote: >>> This provides consistency between io_uring and the respective I/O syscall >>> and avoids having the user of liburing specify the cwd in sqe when working >>> with current working directory, for e.g. the user can directly call with >>> IORING_OP_RENAME instead of IORING_OP_RENAMEAT and providing AT_FDCWD in >>> sqe->fd and sqe->len, similar to syscall interface. >>> >>> This is done for rename, unlink, mkdir, symlink and link in this >>> patch-series. >>> >>> The tests for these opcodes in liburing are present at >>> https://github.com/uarif1/liburing/tree/cwd_opcodes. If the patches are >>> acceptable, I am happy to create a PR in above for the tests. >> >> Can't we just provide prep helpers for them in liburing? >> > > We could add a io_uring_prep_unlink with IORING_OP_UNLINKAT and > AT_FDCWD in liburing. But i guess adding in kernel adds a more > consistent interface? and allows to make calls bypassing liburing > (although i guess people probably don't bypass liburing that much :)) I'm not really aware of much that doesn't use the library, and even those would most likely use the liburing man pages as that's all we have. The kernel API is raw. If you use that, I would expect you to know that you can just use AT_FDCWD! > Making the changes in both kernel and liburing provides more of a > standard interface in my opinion so maybe it looks better. But happy > to just create a PR in liburing only with prep helpers as you > suggested if you think that is better? I don't disagree with that, but it seems silly to waste 5 opcodes on something that is a strict subset of something that is already there. Hence my suggestion would be to just add io_uring_prep_link() etc helpers to make it simpler to use, without having to add 5 extra opcodes.
On 31/05/2022 20:22, Jens Axboe wrote: > On 5/31/22 1:18 PM, Usama Arif wrote: >> >> >> On 31/05/2022 19:58, Jens Axboe wrote: >>> On 5/31/22 12:41 PM, Usama Arif wrote: >>>> This provides consistency between io_uring and the respective I/O syscall >>>> and avoids having the user of liburing specify the cwd in sqe when working >>>> with current working directory, for e.g. the user can directly call with >>>> IORING_OP_RENAME instead of IORING_OP_RENAMEAT and providing AT_FDCWD in >>>> sqe->fd and sqe->len, similar to syscall interface. >>>> >>>> This is done for rename, unlink, mkdir, symlink and link in this >>>> patch-series. >>>> >>>> The tests for these opcodes in liburing are present at >>>> https://github.com/uarif1/liburing/tree/cwd_opcodes. If the patches are >>>> acceptable, I am happy to create a PR in above for the tests. >>> >>> Can't we just provide prep helpers for them in liburing? >>> >> >> We could add a io_uring_prep_unlink with IORING_OP_UNLINKAT and >> AT_FDCWD in liburing. But i guess adding in kernel adds a more >> consistent interface? and allows to make calls bypassing liburing >> (although i guess people probably don't bypass liburing that much :)) > > I'm not really aware of much that doesn't use the library, and even > those would most likely use the liburing man pages as that's all we > have. The kernel API is raw. If you use that, I would expect you to know > that you can just use AT_FDCWD! > >> Making the changes in both kernel and liburing provides more of a >> standard interface in my opinion so maybe it looks better. But happy >> to just create a PR in liburing only with prep helpers as you >> suggested if you think that is better? > > I don't disagree with that, but it seems silly to waste 5 opcodes on > something that is a strict subset of something that is already there. > Hence my suggestion would be to just add io_uring_prep_link() etc > helpers to make it simpler to use, without having to add 5 extra > opcodes. > Thanks, I have created a PR for it on https://github.com/axboe/liburing/pull/588. We can review it there if it makes sense!
On 5/31/22 3:35 PM, Usama Arif wrote: > > > On 31/05/2022 20:22, Jens Axboe wrote: >> On 5/31/22 1:18 PM, Usama Arif wrote: >>> >>> >>> On 31/05/2022 19:58, Jens Axboe wrote: >>>> On 5/31/22 12:41 PM, Usama Arif wrote: >>>>> This provides consistency between io_uring and the respective I/O syscall >>>>> and avoids having the user of liburing specify the cwd in sqe when working >>>>> with current working directory, for e.g. the user can directly call with >>>>> IORING_OP_RENAME instead of IORING_OP_RENAMEAT and providing AT_FDCWD in >>>>> sqe->fd and sqe->len, similar to syscall interface. >>>>> >>>>> This is done for rename, unlink, mkdir, symlink and link in this >>>>> patch-series. >>>>> >>>>> The tests for these opcodes in liburing are present at >>>>> https://github.com/uarif1/liburing/tree/cwd_opcodes. If the patches are >>>>> acceptable, I am happy to create a PR in above for the tests. >>>> >>>> Can't we just provide prep helpers for them in liburing? >>>> >>> >>> We could add a io_uring_prep_unlink with IORING_OP_UNLINKAT and >>> AT_FDCWD in liburing. But i guess adding in kernel adds a more >>> consistent interface? and allows to make calls bypassing liburing >>> (although i guess people probably don't bypass liburing that much :)) >> >> I'm not really aware of much that doesn't use the library, and even >> those would most likely use the liburing man pages as that's all we >> have. The kernel API is raw. If you use that, I would expect you to know >> that you can just use AT_FDCWD! >> >>> Making the changes in both kernel and liburing provides more of a >>> standard interface in my opinion so maybe it looks better. But happy >>> to just create a PR in liburing only with prep helpers as you >>> suggested if you think that is better? >> >> I don't disagree with that, but it seems silly to waste 5 opcodes on >> something that is a strict subset of something that is already there. >> Hence my suggestion would be to just add io_uring_prep_link() etc >> helpers to make it simpler to use, without having to add 5 extra >> opcodes. >> > > Thanks, I have created a PR for it on > https://github.com/axboe/liburing/pull/588. We can review it there if > it makes sense! Sounds good, we'll move it there.