On Fri, Mar 10 2017, Lars Ellenberg wrote: >> --- a/block/blk-core.c >> +++ b/block/blk-core.c >> @@ -1975,7 +1975,14 @@ generic_make_request_checks(struct bio *bio) >> */ >> blk_qc_t generic_make_request(struct bio *bio) >> { >> - struct bio_list bio_list_on_stack; >> + /* >> + * bio_list_on_stack[0] contains bios submitted by the current >> + * make_request_fn. >> + * bio_list_on_stack[1] contains bios that were submitted before >> + * the current make_request_fn, but that haven't been processed >> + * yet. >> + */ >> + struct bio_list bio_list_on_stack[2]; >> blk_qc_t ret = BLK_QC_T_NONE; > > May I suggest that, if you intend to assign something that is not a > plain &(struct bio_list), but a &(struct bio_list[2]), > you change the task member so it is renamed (current->bio_list vs > current->bio_lists, plural, is what I did last year). > Or you will break external modules, silently, and horribly (or, > rather, they won't notice, but break the kernel). > Examples of such modules would be DRBD, ZFS, quite possibly others. > This is exactly what I didn't in my first draft (bio_list -> bio_lists), but then I reverted that change because it didn't seem to be worth the noise. It isn't much noise, sched.h, bcache/btree.c, md/dm-bufio.c, and md/raid1.c get minor changes. But as I'm hoping to get rid of all of those uses, renaming before removing seemed pointless ... though admittedly that is what I did for bioset_create().... I wondered about that too. The example you give later: struct bio_list *tmp = current->bio_list; current->bio_list = NULL; submit_bio() current->bio_list = tmp; won't cause any problem. Whatever lists the parent generic_make_request is holding onto will be untouched during the submit_bio() call, and will be exactly as it expects them when this caller returns. If some out-of-tree code does anything with ->bio_list that makes sense with the previous code, then it will still make sense with the new code. However there will be a few bios that it didn't get too look at. These will all be bios that were submitted by a device further up the stack (closer to the filesystem), so they *should* be irrelevant. I could probably come up with some weird behaviour that might have worked before but now wouldn't quite work the same way. But just fixing bugs can sometimes affect an out-of-tree driver in a strange way because it was assuming those bugs. I hope that I'll soon be able to remove punt_bios_to_rescuer and flush_current_bio_list, after which current->bio_list can really be just a list again. I don't think it is worth changing the name for a transient situation. But thanks for the review - it encouraged me to think though the consequences again and I'm now more confident. I actually now think that change probably wasn't necessary. It is safer though. It ensures that current functionality isn't removed without a clear justification. Thanks, NeilBrown