On Thu, Mar 03, 2016 at 10:17:30AM -0500, Brian Foster wrote: > > + > > + /* > > + * For the last bio, bi_private points to the ioend, so we > > + * need to explicitly end the iteration here. > > + */ > > Do you mean the last bio is pointed to by the ioend? No, bio->bi_private of the last bio points to the ioend. > > @@ -541,10 +457,8 @@ xfs_submit_ioend( > > * at this point in time. > > */ > > error_finish: > > - if (ioend->io_bio) > > - bio_put(ioend->io_bio); > > - ioend->io_error = status; > > - xfs_finish_ioend(ioend); > > + ioend->io_bio->bi_error = status; > > + bio_endio(ioend->io_bio); > > return status; > > bi_end_io is not set here, so what happens to the buffers added to the > ioend in this case? We're not calling the end_io function we should be, good catch and fixed. > Trailing whitespace on the above line. Ok, fixed. > And FWIW, I'm not a huge fan of > open coding both the bio and ioend allocations. It makes it easier to > distinguish the higher level algorithm from all of the structure > initialization noise. It looks to me that alloc_ioend() could remain > mostly as is, using the new bioset allocation, and alloc_ioend_bio() > could be inlined and renamed to something like init_bio_from_bh() or > some such. Hmm, not a huge fan of these single use function in general, but I'll see if I can do something sensible. > I'm trying to make sure I grok how this works without knowing much about > the block layer. So we chain the current bio to the new one, the latter > becoming the parent, and submit the old one. It looks to me that this > could result in bio_endio() on the parent, which seems fishy... what am > I missing? IOW, is it safe to submit bios like this before the entire > chain is created? Ignoring this for now and jumping to the next reply.. > > + out_free_ioend_bioset: > > + bioset_free(xfs_ioend_bioset); > > Space before tab ^. Fixed. > > + bioset_free(xfs_ioend_bioset); > > Space before tab ^. Fixed. _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
