On Tue, Aug 28, 2012 at 01:31:48PM -0700, Tejun Heo wrote: > Hello, Kent. > > On Tue, Aug 28, 2012 at 10:37:30AM -0700, Kent Overstreet wrote: > > Reusing bios is something that's been highly frowned upon in the past, > > but driver code keeps doing it anyways. If it's going to happen anyways, > > we should provide a generic method. > > > > This'll help with getting rid of bi_destructor - drivers/block/pktcdvd.c > > was open coding it, by doing a bio_init() and resetting bi_destructor. > > Better to explain why some bio fields are re-ordered and why that > shouldn't make things worse cacheline-wise? Well it may (struct bio is what, 3 or 4 cachelines now?) but even on ridiculous million iop devices struct bio just isn't hot enough for this kind of stuff to show up in the profiles... and I've done enough profiling to be pretty confident of that. So um, if there was anything to say performance wise I would, but to me it seems more that there isn't. (pruning struct bio would have more of an effect performance wise, which you know is something I'd like to do.) > > > +void bio_reset(struct bio *bio) > > +{ > > Function comment explaining what it does and why it does what it does > with integrity / bi_css / whatnot? Yeah, good idea. > > > + unsigned long flags = bio->bi_flags & (~0UL << BIO_RESET_BITS); > > + > > + if (bio_integrity(bio)) > > + bio_integrity_free(bio, bio->bi_pool); > > + > > + bio_disassociate_task(bio); > > Is this desirable? Why? *twitch* I should've thought more when I made that change. It occurs to me now that we specifically talked about that and decided to do it the other way - when I changed that I was just looking at bio_free() and decided they needed to be symmetrical. I still think they should be symmetrical, but if that's true bi_ioc and bi_css need to be moved, and also bio_disassociate_task() should be getting called from bio_free(), not bio_put(). Were you the one that added that call? I know you've been working on that area of the code recently. Sticking it in bio_put() instead of bio_free() seems odd to be, and they're completely equivalent now that bio_free() is only called from bio_put() (save one instance I should probably fix). -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel