AIO: remove the kaiocb->bio linkage
ClosedPublic
Actions

Authored by asomers on Dec 19 2020, 5:21 PM.

Details

Reviewers

kib
jhb

Summary

AIO: remove the kaiocb->bio linkage

Vectored aio will require each aiocb to be associated with multiple bios, so
we can't store a link to the latter from the former. But we don't really
need to. aio_biowakeup already knows the bio it's using, and the other
fields can be stored within the bio itself.

Also, remove the unused kaiocb.backend2 field.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 35579
Build 32479: arc lint + arc unit

Event Timeline

asomers created this revision.Dec 19 2020, 5:21 PM

Herald added a subscriber: imp. · View Herald TranscriptDec 19 2020, 5:21 PM

asomers requested review of this revision.Dec 19 2020, 5:21 PM

Harbormaster completed remote builds in B35541: Diff 80956.Dec 19 2020, 5:21 PM

kib added inline comments.Dec 20 2020, 4:09 AM

sys/kern/vfs_aio.c
1310	I did a lot of code reading to convince myself that no other code does the similar trick. But then it is not clear to me why do you need to assign bio_ma and bio_ma_n. You have pbuf in bio_caller1, and this provides the needed info. aio_biowakeup still needs to distinguish unmapped vs mapped (pbuf used) cases. If you use pbuf instead of using bio_ma* without BIO_UNMAPPED set, it fits with the current code assumptions.
2350	Perhaps assert that bio_ma_n is <= atop(maxphys) + 1.
sys/sys/aio.h
140–141	What is the sense in keeping this empty structure ? (Not to mention that it has strange ABI rules)

asomers marked an inline comment as done.Dec 21 2020, 5:26 PM

asomers added inline comments.

sys/kern/vfs_aio.c
1310	Ok, I can replace `bp->bio_ma` with `pbuf->b_pages`. But I still don't see where `pbuf` stores `npages`. Unless I duplicate a lot of logic from `vm_fault_quick_hold_pages`, I think I have to store `npages` somewhere, like in `bp->bio_ma_n`. Or could you enlighten me?
sys/sys/aio.h
140–141	Because I plan to almost immediately add some more fields back in, in D27624. But if you'd prefer, I can delete it for now and re-add it in that future commit.
151	Should I bump `__FreeBSD_version__` on account of removing `backend2`? It seems unlikely that there are any out-of-tree consumers.

kib added inline comments.Dec 21 2020, 7:20 PM

sys/kern/vfs_aio.c
1310	It does not store npages, but there are a lot of places you can reuse safely. Most obvious and natural is b_npages that is not used by pbuf machinery at all.
sys/sys/aio.h
140–141	No need to delete then, but a short note in the commit message would be useful.
151	I do not believe the bump needed (or that it is too useful).

Don't override bp->bio_ma and bp->bio_ma_n

Override pbuf->b_npages instead.

Harbormaster completed remote builds in B35579: Diff 81010.Dec 21 2020, 8:48 PM

kib accepted this revision.Dec 21 2020, 9:22 PM

This revision is now accepted and ready to land.Dec 21 2020, 9:22 PM

jhb added inline comments.Dec 21 2020, 11:08 PM

sys/sys/aio.h
140–141	I think it might be best to delete and bring back. Empty structs are technically undefined in C. You could perhaps add a "long dummy" member for now that you remove in your later change perhaps, but just removing the struct for now and bringing it back later seems cleanest to me.