On 5/30/23 10:06?AM, Kent Overstreet wrote: > On Tue, May 30, 2023 at 08:22:50AM -0600, Jens Axboe wrote: >> On 5/26/23 2:44?PM, Kent Overstreet wrote: >>> On Fri, May 26, 2023 at 08:35:23AM -0600, Jens Axboe wrote: >>>> On 5/25/23 3:48?PM, Kent Overstreet wrote: >>>>> Jens, here's the full series of block layer patches needed for bcachefs: >>>>> >>>>> Some of these (added exports, zero_fill_bio_iter?) can probably go with >>>>> the bcachefs pull and I'm just including here for completeness. The main >>>>> ones are the bio_iter patches, and the __invalidate_super() patch. >>>>> >>>>> The bio_iter series has a new documentation patch. >>>>> >>>>> I would still like the __invalidate_super() patch to get some review >>>>> (from VFS people? unclear who owns this). >>>> >>>> I wanted to check the code generation for patches 4 and 5, but the >>>> series doesn't seem to apply to current -git nor my for-6.5/block. >>>> There's no base commit in this cover letter either, so what is this >>>> against? >>>> >>>> Please send one that applies to for-6.5/block so it's a bit easier >>>> to take a closer look at this. >>> >>> Here you go: >>> git pull https://evilpiepirate.org/git/bcachefs.git block-for-bcachefs >> >> Thanks >> >> The re-exporting of helpers is somewhat odd - why is bcachefs special >> here and needs these, while others do not? > > It's not iomap based. > >> But the main issue for me are the iterator changes, which mostly just >> seems like unnecessary churn. What's the justification for these? The >> commit messages don;t really have any. Doesn't seem like much of a >> simplification, and in fact it's more code than before and obviously >> more stack usage as well. > > I need bio_for_each_folio(). > > The approach taken by the bcachefs IO paths is to first build up bios, > then walk the extents btree to determine where to send them, splitting > as needed. > > For reading into the page cache we additionally need to initialize our > private state based on what we're reading from that says what's on > disk (unallocated, reservation, or normal allocation) and how many > replicas. This is used for both i_blocks accounting and for deciding > when we need to get a disk reservation. Since we're doing this post > split, it needs bio_for_each_folio, not the _all variant. > > Yes, the iterator changes are a bit more code - but it's split up into > better helpers now, the pointer arithmetic before was a bit dense; I > found the result to be more readable. I'm surprised at more stack > usage; I would have expected _less_ for bio_for_each_page_all() since > it gets rid of a pointer into the bvec_iter_all. How did you measure > that? Sorry typo, I meant text. Just checked stack and it looks identical, but things like blk-map grows ~6% more text, and bio ~3%. Didn't check all of them, but at least those two are consistent across x86-64 and aarch64. Ditto on the data front. Need to take a closer look at where exactly that is coming from, and what that looks like. -- Jens Axboe
