On 6/2/22 2:12 AM, Christoph Hellwig wrote: > On Wed, Jun 01, 2022 at 10:13:34AM -0400, Mike Snitzer wrote: >> Please take the time to look at the code and save your judgement until >> you do. That said, I'm not in love with the complexity of how DM >> handles bioset initialization. But both you and Jens keep taking >> shots at DM for doing things wrong without actually looking. > > I'm not taking shots. I'm just saying we should kill this API. In > the worse case the caller can keep track of if a bioset is initialized, > but in most cases we should be able to deduct it in a nicer way. Yeah ditto, it's more an observation on needing something like this_foo_was_initialized() is just a bad way to program it to begin with. The caller should know this already, rather than us needing functions and state in the struct to tell you about it. >> DM uses bioset_init_from_src(). Yet you've both assumed double frees >> and such (while not entirely wrong your glossing over the detail that >> there is intervening reinitialization of DM's biosets between the >> bioset_exit()s) > > And looking at the code, that use of bioset_init_from_src is completely > broken. It does not actually preallocated anything as intended by > dm (maybe that isn't actually needed) but just uses the biosets in > dm_md_mempools as an awkward way to carry parameters. And completely > loses bringing over the integrity allocations. And no, this is not > intended as a "cheap shot" against Jens who did that either.. > > This is what I think should fix this, and will allow us to remove > bioset_init_from_src which was a bad idea from the start: Based on a quick look, seems good to me. -- Jens Axboe
