* [RFC] fixed files @ 2020-02-08 13:28 Pavel Begunkov 2020-02-08 20:15 ` Jens Axboe 0 siblings, 1 reply; 5+ messages in thread From: Pavel Begunkov @ 2020-02-08 13:28 UTC (permalink / raw) To: Jens Axboe, io-uring Hi, As you remember, splice(2) needs two fds, and it's a bit of a pain finding a place for the second REQ_F_FIXED_FILE flag. So, I was thinking, can we use the last (i.e. sign) bit to mark an fd as fixed? A lot of userspace programs consider any negative result of open() as an error, so it's more or less safe to reuse it. e.g. fill_sqe(fd) // is not fixed fill_sqe(buf_idx | LAST_BIT) // fixed file -- Pavel Begunkov ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] fixed files 2020-02-08 13:28 [RFC] fixed files Pavel Begunkov @ 2020-02-08 20:15 ` Jens Axboe 2020-02-09 12:18 ` Pavel Begunkov 0 siblings, 1 reply; 5+ messages in thread From: Jens Axboe @ 2020-02-08 20:15 UTC (permalink / raw) To: Pavel Begunkov, io-uring On 2/8/20 6:28 AM, Pavel Begunkov wrote: > Hi, > > As you remember, splice(2) needs two fds, and it's a bit of a pain > finding a place for the second REQ_F_FIXED_FILE flag. So, I was > thinking, can we use the last (i.e. sign) bit to mark an fd as fixed? A > lot of userspace programs consider any negative result of open() as an > error, so it's more or less safe to reuse it. > > e.g. > fill_sqe(fd) // is not fixed > fill_sqe(buf_idx | LAST_BIT) // fixed file Right now we only support 1024 fixed buffers anyway, so we do have some space there. If we steal a bit, it'll still allow us to expand to 32K of fixed buffers in the future. It's a bit iffy, but like you, I don't immediately see a better way to do this that doesn't include stealing an IOSQE bit or adding a special splice flag for it. Might still prefer the latter, to be honest... -- Jens Axboe ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] fixed files 2020-02-08 20:15 ` Jens Axboe @ 2020-02-09 12:18 ` Pavel Begunkov 2020-02-09 17:04 ` Jens Axboe 0 siblings, 1 reply; 5+ messages in thread From: Pavel Begunkov @ 2020-02-09 12:18 UTC (permalink / raw) To: Jens Axboe, io-uring On 2/8/2020 11:15 PM, Jens Axboe wrote: > On 2/8/20 6:28 AM, Pavel Begunkov wrote: >> Hi, >> >> As you remember, splice(2) needs two fds, and it's a bit of a pain >> finding a place for the second REQ_F_FIXED_FILE flag. So, I was >> thinking, can we use the last (i.e. sign) bit to mark an fd as fixed? A >> lot of userspace programs consider any negative result of open() as an >> error, so it's more or less safe to reuse it. >> >> e.g. >> fill_sqe(fd) // is not fixed >> fill_sqe(buf_idx | LAST_BIT) // fixed file > > Right now we only support 1024 fixed buffers anyway, so we do have some > space there. If we steal a bit, it'll still allow us to expand to 32K of > fixed buffers in the future. > > It's a bit iffy, but like you, I don't immediately see a better way to > do this that doesn't include stealing an IOSQE bit or adding a special > splice flag for it. Might still prefer the latter, to be honest... "fixed" is clearly a per-{fd,buffer} attribute. If I'd now design it from the scratch, I would store fixed-resource index in the same field as fds and addr (but not separate @buf_index), and have per-resource switch-flag somewhere. And then I see 2 convenient ways: 1. encode the fixed bit into addr and fd, as supposed above. 2. Add N generic IOSQE_FIXED bits (i.e. IOSQE_FIXED_RESOURSE{1,2,...}), which correspond to resources (fd, buffer, etc) in order of occurrence in an sqe. I wouldn't expect having more than 3-4 flags. And then IORING_OP_{READ,WRITE}_FIXED would have been the same opcode as the corresponding non-fixed version. But backward-compatibility is a pain. -- Pavel Begunkov ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] fixed files 2020-02-09 12:18 ` Pavel Begunkov @ 2020-02-09 17:04 ` Jens Axboe 2020-02-09 17:53 ` Pavel Begunkov 0 siblings, 1 reply; 5+ messages in thread From: Jens Axboe @ 2020-02-09 17:04 UTC (permalink / raw) To: Pavel Begunkov, io-uring On 2/9/20 5:18 AM, Pavel Begunkov wrote: > On 2/8/2020 11:15 PM, Jens Axboe wrote: >> On 2/8/20 6:28 AM, Pavel Begunkov wrote: >>> Hi, >>> >>> As you remember, splice(2) needs two fds, and it's a bit of a pain >>> finding a place for the second REQ_F_FIXED_FILE flag. So, I was >>> thinking, can we use the last (i.e. sign) bit to mark an fd as fixed? A >>> lot of userspace programs consider any negative result of open() as an >>> error, so it's more or less safe to reuse it. >>> >>> e.g. >>> fill_sqe(fd) // is not fixed >>> fill_sqe(buf_idx | LAST_BIT) // fixed file >> >> Right now we only support 1024 fixed buffers anyway, so we do have some >> space there. If we steal a bit, it'll still allow us to expand to 32K of >> fixed buffers in the future. >> >> It's a bit iffy, but like you, I don't immediately see a better way to >> do this that doesn't include stealing an IOSQE bit or adding a special >> splice flag for it. Might still prefer the latter, to be honest... > > "fixed" is clearly a per-{fd,buffer} attribute. If I'd now design it > from the scratch, I would store fixed-resource index in the same field > as fds and addr (but not separate @buf_index), and have per-resource > switch-flag somewhere. And then I see 2 convenient ways: > > 1. encode the fixed bit into addr and fd, as supposed above. > > 2. Add N generic IOSQE_FIXED bits (i.e. IOSQE_FIXED_RESOURSE{1,2,...}), > which correspond to resources (fd, buffer, etc) in order of occurrence > in an sqe. I wouldn't expect having more than 3-4 flags. > > And then IORING_OP_{READ,WRITE}_FIXED would have been the same opcode as > the corresponding non-fixed version. But backward-compatibility is a pain. It's always much easier looking back, hindsight is much clearer. I'd also expand the sqe flags bits to 16 at least, but oh well. I do think that for this particular case we add a SPLICE_F_FD1_FIXED and ditto for fd2, and just have the direct splice/vmsplice syscalls reject them as invalid. Both splice and vmsplice -EINVAL for unknown flags, which makes this possible. That seems cleaner to me than trying to shoe-horn this information into the sqe itself, and it can easily be done as a prep patch to adding splice support. -- Jens Axboe ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC] fixed files 2020-02-09 17:04 ` Jens Axboe @ 2020-02-09 17:53 ` Pavel Begunkov 0 siblings, 0 replies; 5+ messages in thread From: Pavel Begunkov @ 2020-02-09 17:53 UTC (permalink / raw) To: Jens Axboe, io-uring [-- Attachment #1.1: Type: text/plain, Size: 2657 bytes --] On 09/02/2020 20:04, Jens Axboe wrote: > On 2/9/20 5:18 AM, Pavel Begunkov wrote: >> On 2/8/2020 11:15 PM, Jens Axboe wrote: >>> On 2/8/20 6:28 AM, Pavel Begunkov wrote: >>>> Hi, >>>> >>>> As you remember, splice(2) needs two fds, and it's a bit of a pain >>>> finding a place for the second REQ_F_FIXED_FILE flag. So, I was >>>> thinking, can we use the last (i.e. sign) bit to mark an fd as fixed? A >>>> lot of userspace programs consider any negative result of open() as an >>>> error, so it's more or less safe to reuse it. >>>> >>>> e.g. >>>> fill_sqe(fd) // is not fixed >>>> fill_sqe(buf_idx | LAST_BIT) // fixed file >>> >>> Right now we only support 1024 fixed buffers anyway, so we do have some >>> space there. If we steal a bit, it'll still allow us to expand to 32K of >>> fixed buffers in the future. >>> >>> It's a bit iffy, but like you, I don't immediately see a better way to >>> do this that doesn't include stealing an IOSQE bit or adding a special >>> splice flag for it. Might still prefer the latter, to be honest... >> >> "fixed" is clearly a per-{fd,buffer} attribute. If I'd now design it >> from the scratch, I would store fixed-resource index in the same field >> as fds and addr (but not separate @buf_index), and have per-resource >> switch-flag somewhere. And then I see 2 convenient ways: >> >> 1. encode the fixed bit into addr and fd, as supposed above. >> >> 2. Add N generic IOSQE_FIXED bits (i.e. IOSQE_FIXED_RESOURSE{1,2,...}), >> which correspond to resources (fd, buffer, etc) in order of occurrence >> in an sqe. I wouldn't expect having more than 3-4 flags. >> >> And then IORING_OP_{READ,WRITE}_FIXED would have been the same opcode as >> the corresponding non-fixed version. But backward-compatibility is a pain. > > It's always much easier looking back, hindsight is much clearer. I'd also > expand the sqe flags bits to 16 at least, but oh well. Totally agree. And I didn't meant I could have done better BTW, we can extend sqe flags into free bits at the end of sqe, would need a bit of packing/unpacking though. e.g. int flags = sqe1_flags | (sqe2_flags << 8) > I do think that for this particular case we add a SPLICE_F_FD1_FIXED and > ditto for fd2, and just have the direct splice/vmsplice syscalls reject > them as invalid. Both splice and vmsplice -EINVAL for unknown flags, > which makes this possible. Yes, I remember that from the splice thread. > That seems cleaner to me than trying to shoe-horn this information into > the sqe itself, and it can easily be done as a prep patch to adding > splice support. > -- Pavel Begunkov [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-02-09 17:54 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-08 13:28 [RFC] fixed files Pavel Begunkov 2020-02-08 20:15 ` Jens Axboe 2020-02-09 12:18 ` Pavel Begunkov 2020-02-09 17:04 ` Jens Axboe 2020-02-09 17:53 ` Pavel Begunkov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).