On 9/9/2019 5:54 PM, Alberto Garcia wrote: > On Fri 06 Sep 2019 06:01:14 PM CEST, Anton Nefedov wrote: >> This adds some protection from accounting uninitialized cookie. >> That is, block_acct_failed/done without previous block_acct_start; >> in that case, cookie probably holds values from previous operation. >> >> (Note: it might also be uninitialized holding garbage value and there >> is still "< BLOCK_MAX_IOTYPE" assertion for that. So >> block_acct_failed/done without previous block_acct_start should be >> used with caution.) >> >> Currently this is particularly useful in ide code where it's hard to >> keep track whether the request started accounting or not. For example, >> trim requests do the accounting separately. > > Sorry if I'm understanding it wrong, but it sounds like you know that > there's a bug in the ide code (where you call block_acct_done() without > having it initialized it first), and the purpose of the this patch is to > hide the bug ? >
hi, not really; in the existing code, I can't see block_acct_done() without block_acct_start(), but there might be double-accounting though; e.g. ide_atapi_cmd_read_dma_cb(): it can account the same operation twice like ide_handle_rw_error(); goto eot; block_acct_failed(); The patch should solve it. The commit message is misleading, sorry. I'll change to: > Each block_acct_done/failed call is designed to correspond to a > previous block_acct_start call, which initializes the stats cookie. > However sometimes it is not the case, e.g. some error paths might > report the same cookie twice because it is hard to accurately track if > the cookie was reported yet or not. > This patch cleans the cookie after report. > (Note: block_acct_failed/done without a previous block_acct_start at > all should be avoided. Uninitialized cookie might hold a garbage value > and there is still "< BLOCK_MAX_IOTYPE" assertion for that) > It will be particularly useful in ide code where it's hard to > keep track whether the request done its accounting or not: in the > following patch of the series, trim requests will do the accounting > separately. /Anton