Page MenuHomeFreeBSD

AIO: remove the kaiocb->bio linkage
ClosedPublic

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 OK
Unit
No Unit Test Coverage
Build Status
Buildable 35579
Build 32479: arc lint + arc unit

Event Timeline

sys/kern/vfs_aio.c
1309

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.

2349

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 added inline comments.
sys/kern/vfs_aio.c
1309

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.

sys/kern/vfs_aio.c
1309

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.

This revision is now accepted and ready to land.Dec 21 2020, 9:22 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.

sys/sys/aio.h
140–141

Fair enough. I'll do that, once we're allowed to commit again.