On 6/18/20 5:04 AM, Jan Kara wrote: >> The attempt_merge function is also responsible for the discard merging >> which doesn't have any direction specified in ELEVATOR_XXX identifiers. >> In this patch we care unconditionally generating back merge event >> irrespective of the req_op. >> >> Do we need to either generate event iff ELEVATOR_BACK_MERGE true case or >> add another trace point for discard ? given that it may lead to >> confusion since elevator flags for the discard doesn't specify the >> direction. > attempt_merge() is always called so that 'req' is always the request with > the lower sector, 'next' is the request with a higher sector, and 'next' is > discarded and 'req' is updated. So attempt_merge() always performs only one > direction of a merge and I don't think it makes any sence to distinguish > back merges and forward merges in this case. So discards don't need any > special treatment either AFAICT. > Please disregard my comment regarding adding a separate tracpoint for discard. I understand the code and that is what I'm saying it will only back-merge. The concern is just like we have ELEVATOR_BACK_MERGE used in attempt_merge, we should also have ELEVATOR_DISCARD_BACK_MERGE and use it the attempt_merge() then this tracepoint will map to the back-merge which your patch does with the use of BLK_TC_BACKMERGE nothing needed in your patch to change regarding tracepoint. > Honza > > -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
