On 06/13/2017 03:52 AM, Kevin Wolf wrote: > Am 12.06.2017 um 23:20 hat John Snow geschrieben: >> I noticed while debugging an unrelated test that our use of the null >> driver has a habit of making functions like find_image_format trigger a >> lot of uninitialized memory errors in valgrind, because it will return a >> successful read without actually touching the buffer. >> >> I see that in March there was a bit of a debate over whether or not the >> null driver SHOULD zero-write memory for performance reasons, but when >> this option isn't specified, the semantics of read are arguably broken. >> >> This isn't terribly important to fix, but for actual debugging purposes >> it's nice to not have valgrind screaming at you with spurious junk. A >> few options: >> >> (1) Admit that it's weird to allow reads to succeed with null-co driver. >> Annoy people who apparently wanted performance for their do-nothing >> driver and force the initialization of memory. >> >> --or-- >> >> (1a) Use valgrind macros to pretend the memory has been initialized. >> >> (2) Band-aid find_image_format to zero-initialize memory. Wonder if >> there are other usages of read() getting called from tests that utilize >> null-co that will make debugging difficult in other contexts. >> >> (3) Edit any tests to always use the zero option when using the null >> driver, and then periodically keep updating this option in a losing >> battle over time. >> >> (4) Find a way to adjust return value of null-co's read implementation >> to return 0 instead of a lie over the number of bytes it read/wrote. >> Correctly represents "No bytes written, but no error occurred either." > > The block layer doesn't support short reads. Therefore, the return value > is 0/-errno even today, the byte count isn't returned anywhere. >
Run the blockjob transaction test and take a look at what the read is returning in find_image_format. It's 512! I didn't look further than that, I assumed it was intentional. >> (5) Shut up, John, really, who cares? Please shut up. You are the only >> idiot who would even think to use valgrind ever, and we just don't like >> you that much. > > (6) Make read-zeroes=on the default. Gives people worse performance if > they expect that null-co:// without any options is the fastest thing > possible, but it has defined behaviour by default which seems good. > > Whatever solution we choose, I think it should be local to the null > driver, i.e. I don't like (2). > Neither do I, really. Having read all of the responses, I think I'd like your approach of making read-zeroes the default, and people who "really know what they want" can turn it off and enter the danger zone if they'd like to.
