On 3/11/20 22:35, santosh.shilimkar@xxxxxxxxxx wrote: > On 3/10/20 9:48 PM, zerons wrote: >> >> >> On 3/11/20 01:53, santosh.shilimkar@xxxxxxxxxx wrote: >>> On 3/6/20 4:11 AM, zerons wrote: >>>> >>>> >>>> On 2/28/20 02:10, santosh.shilimkar@xxxxxxxxxx wrote: >>>>> >>>>>>> On 18 Feb 2020, at 14:13, zerons <sironhide0null@xxxxxxxxx> wrote: >>>>>>> >>>>>>> Hi, all >>>>>>> >>>>>>> In net/rds/rdma.c >>>>>>> (https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/net/rds/rdma.c?h=v5.5.3*n419__;Iw!!GqivPVa7Brio!OwwQCLtjDsKmhaIz0sfaOVSuC4ai5t5_FgB7yqNExGOCBtACtIGLF61NNJyqSDtIAcGoPg$ ), >>>>>>> there may be a race condition between rds_rdma_unuse() and rds_free_mr(). >>>>>>> >>>>> Hmmm.. I didn't see email before in my inbox. Please post questions/patches on netdev in future which is the correct mailing list. >>>>> >>>>>>> It seems that this one need some specific devices to run test, >>>>>>> unfortunately, I don't have any of these. >>>>>>> I've already sent two emails to the maintainer for help, no response yet, >>>>>>> (the email address may not be in use). >>>>>>> >>>>>>> 0) in rds_recv_incoming_exthdrs(), it calls rds_rdma_unuse() when receive an >>>>>>> extension header with force=0, if the victim mr does not have RDS_RDMA_USE_ONCE >>>>>>> flag set, then the mr would stay in the rbtree. Without any lock, it tries to >>>>>>> call mr->r_trans->sync_mr(). >>>>>>> >>> MR won't stay in the rbtree with force flag. If the MR is used or >>> use_once is set in both cases its removed from the tree. >>> See "if (mr->r_use_once || force)" >>> >> >> Sorry, I may misunderstand. Did you mean that if the MR is *used*, >> it is removed from the tree with or without the force flag in >> rds_rdma_unuse(), even when r_use_once is not set? >> > Once the MR is being used with use_once semantics it gets removed with or without remote side indicating it via extended header. use_once > optimization was added later. The base behavior is once the MR is > used by remote and same information is sent via extended header, > it gets cleaned up with force flag. Force flag ignores whether > its marked as used_once or not. > Sorry, I am still confused. I check the code again. The rds_rdma_unuse() is called in two functions, rds_recv_incoming_exthdrs() and rds_sendmsg(). In rds_sendmsg(), it calls rds_rdma_unuse() *with* force flag only when the user included a RDMA_MAP cmsg *and* sendmsg() is failed. In rds_recv_incoming_exthdrs(), the force is *false*. So we can consider the rds_rdma_unuse() called *without* force flag. Then I go check where r_use_once can be set. __rds_rdma_map() rds_get_mr() rds_setsockopt() rds_get_mr_for_dest() rds_setsockopt() rds_cmsg_rdma_map() rds_cmsg_send() rds_sendmsg() It seems to me that r_use_once is controlled by user applications. I also wonder if we can ensure that the MR found in rds_rdma_unuse() gets removed, then "if (mr->r_use_once || force)" doesn't make any sense. Sorry to keep bothering you with my questions. I wish I had such a device that I can test it on. Best regards,
