SoftRoCE / rxe-dev

Development Repository for RXE
Other
132 stars 56 forks source link

Replace net_info array with a linked list #8

Closed Kamalheib closed 9 years ago

Kamalheib commented 9 years ago

Need to replace the net_info array in rxe_net.h with a linked list and every place that accessing the array change it to accessing the linked list.

After doing this no need for RXE_MAX_IF_INDEX macro

psebexen commented 9 years ago

New code has been pushed to reflect kernel version in 'upstream-submission' branch. Compiles successfully, loads and unloads successfully, passes ibv_rc_pingpong (latter only tested with server and client as same machine due to limitations of my setup).

psebexen commented 9 years ago

New code has been pushed and tested across multiple machines. Please let me know if you have any comments/questions about the changes.

Kamalheib commented 9 years ago

Hi Paul, I added some comments, please take a look https://github.com/SoftRoCE/rxe-dev/commit/3a7308c8af93362ece6c511c77b4693a18237c9d

psebexen commented 9 years ago

Thanks Kamal. I agree with all of your suggested changes; they should be corrected now in 327a12cdadeae7ef27ca05884c41f7b4e67ef854.

psebexen commented 9 years ago

Sorry; one more correction in 02ffc1b8b01c394a86b157844779afc5327d6877 as well.

Kamalheib commented 9 years ago

Could you please amend all the fixes to [1] commit, this will help in the review process and make the git more clear.

[1] - https://github.com/SoftRoCE/rxe-dev/commit/3a7308c8af93362ece6c511c77b4693a18237c9d

psebexen commented 9 years ago

They are all combined in 2daaa9a8e961f4832b2e49399bd48378e7e329d6. As a note, I had to delete two public commits to do this amendment (which is generally not recommended), so you may have to do a local reset to pull the new commit successfully.

psebexen commented 9 years ago

Thank you for the new comments, Kamal. I will make the new changes and commit. I have two questions for you: 1) Should I add the new commit as usual, or do you prefer I amend the last one again to keep the history clean? 2) Is it safe to eliminate the lines explicitly setting newly initialized list item struct fields to 0 by replacing kmalloc with kzalloc? (I added them originally to guard against exceptions, e.g. in the net_info_list_add() function)

Kamalheib commented 9 years ago

Hello Paul,

Related to your questions: 1- I prefer to amend the changes so the history will be clean. 2- I'm sorry i didn't understand your questions, could you please give an example from the code.

Thanks, Kamal

psebexen commented 9 years ago

1 - Ok; thanks.

2 - Sorry for the confusing wording; I am curious if I can replace this code from the net_info list add() function:

net_info_item = kmalloc(sizeof(*net_info_item), GFP_KERNEL);
net_info_item->ifindex = index;
net_info_item->rxe = NULL;
net_info_item->status = 0;
net_info_item->port = 0;
net_info_item->ndev = NULL;
INIT_LIST_HEAD(&net_info_item->list);

with the following:

net_info_item = kzalloc(sizeof(*net_info_item), GFP_KERNEL);
net_info_item->ifindex = index;
INIT_LIST_HEAD(&net_info_item->list);

It appears kzalloc will do the "zeroing" automatically and the resulting code looks cleaner.

Kamalheib commented 9 years ago

I think that in this case you need to use kzalloc

psebexen commented 9 years ago

Outstanding corrections have been made and branch is rebased from 'master' to 'master-next'. Latest commit here: 1943c23ac79ed0b6142370218bcd64bb1c205010.