Hello, On Mon, 23 Feb 2015, Alex Gartrell wrote: > We have a custom scheduler that is failing some atomic allocations You can use GFP_KERNEL from add_dest method. > sometimes, and it was made pretty hard to discover by the fact that we more > or less drop the return code on the floor. This is especially nasty, as > the ipvsadm -ln invocation shows the scheduler to be as we wish it were > instead of as it is. > > This is an RFC because I have no idea what the best way to proceed with the > upd_dest invocation is. I could clone all of the state and simply restore > it, but I figured one of you might have a better idea. Can a new create_dest(svc, bool for_update) method solve such problem? Such modifications are serialized, so create_dest can attach new structure to sched_data just before the add_dest/upd_dest call. IPVS does not clone dests on update but scheduler may need this for RCU update. create_dest should be called early in __ip_vs_update_dest. Note that kfree(dest) is not enough, see ip_vs_dest_free(), more fields may be allocated before __ip_vs_update_dest is reached. Patch 2: dest_p is not needed, this patch is ok Patch 3: In the __ip_vs_update_dest change you are missing that ip_vs_rs_hash was called. In fact, I think we should call __ip_vs_unlink_dest(svc, dest, 0) and __ip_vs_del_dest(svc->net, dest, false) from ip_vs_add_dest() when both __ip_vs_update_dest(, 1) and ip_vs_new_dest() fail. This should work because add_dest call is the last thing we do on ADD, so the DEL operation but without calling del_dest should be ok. Such change invalidates Patch 1. Why above solution is preferred: At first look, scheduler that can return error from add_dest should not provide this dest to RCU readers via its schedule method and kfree(dest) looks ok. The problem is with the ip_vs_has_real_service() call and the ip_vs_rs_hash() call we do before add_dest, dest still can be accessed by RCU readers. For this reason we can not just kfree(dest). Patch 1: Not needed if patch 3 is changed Patch 4: As for del_dest result, failure should not happen, we should be able to flush dests on module cleanup. I think, there should not be any allocations in this case. Regards -- Julian Anastasov <ja@xxxxxx> -- To unsubscribe from this list: send the line "unsubscribe lvs-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
