On Tue, Jan 10, 2012 at 10:20 PM, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > On Sun, Jan 8, 2012 at 10:01 PM, Yinghai Lu <yinghai@xxxxxxxxxx> wrote: >> >> Spent some time today, try to produce some patches according to your direction. >> >> Please check them to see if it is with your thoughts. > > Get rid of "insert_list_before()". That's a particularly bad > implementation of "list_add_tail()", which we already have. And > list_add_tail() generates better code, and supports the proper full > list debugging. > > (Think about it: adding something "before" an entry in a doubly-linked > circular list is the same as adding it at the tail of the list - it's > circular!) Indeed. could just use list_add_tail instead. > > The other patches look reasonable, but I only gave them a quick look. > Although why is "free_list()" a #define rather than a real function > (apart from the obvious reason that it used to be that before too..) > > Oh, and I just noticed that you fixed that in the "merge struct" > patch, which is good, except I think it would be much better to aim to > do *one* thing per patch, instead of mixing them up like that. ok, will separate to another patch about free_list changes. > > .. and when you merged the "_x" resource struct, you still left the > variables with the "_x" names.. Do you really need dev_res and > dev_res_x variables? sure, will drop _x. Thanks Yinghai -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html