{"action":"created","issue":{"url":"https://api.github.com/repos/openebs/zfs/issues/82","repository_url":"https://api.github.com/repos/openebs/zfs","labels_url":"https://api.github.com/repos/openebs/zfs/issues/82/labels{/name}","comments_url":"https://api.github.com/repos/openebs/zfs/issues/82/comments","events_url":"https://api.github.com/repos/openebs/zfs/issues/82/events","html_url":"https://github.com/openebs/zfs/pull/82","id":338554414,"node_id":"MDExOlB1bGxSZXF1ZXN0MTk5NDU0NjE4","number":82,"title":"[TA1652] Lockless refcount increment and decrement","user":{"login":"satbirchhikara","id":34478047,"node_id":"MDQ6VXNlcjM0NDc4MDQ3","avatar_url":"https://avatars2.githubusercontent.com/u/34478047?v=4","gravatar_id":"","url":"https://api.github.com/users/satbirchhikara","html_url":"https://github.com/satbirchhikara","followers_url":"https://api.github.com/users/satbirchhikara/followers","following_url":"https://api.github.com/users/satbirchhikara/following{/other_user}","gists_url":"https://api.github.com/users/satbirchhikara/gists{/gist_id}","starred_url":"https://api.github.com/users/satbirchhikara/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/satbirchhikara/subscriptions","organizations_url":"https://api.github.com/users/satbirchhikara/orgs","repos_url":"https://api.github.com/users/satbirchhikara/repos","events_url":"https://api.github.com/users/satbirchhikara/events{/privacy}","received_events_url":"https://api.github.com/users/satbirchhikara/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2018-07-05T12:30:57Z","updated_at":"2018-07-06T05:32:24Z","closed_at":null,"author_association":"NONE","pull_request":{"url":"https://api.github.com/repos/openebs/zfs/pulls/82","html_url":"https://github.com/openebs/zfs/pull/82","diff_url":"https://github.com/openebs/zfs/pull/82.diff","patch_url":"https://github.com/openebs/zfs/pull/82.patch"},"body":"Signed-off-by: satbir <satbir.chhikara@gmail.com>\r\n\r\nHi,\r\n\r\nI have made changes to implement lockless increment and decrement of refcount on zinfo object. I have done frist round of testing. Before I do more robust Unit testing ( with automation), want take your opinion if you see any issue with current implementation. Little details about zinfo refcounting ...\r\n1. There are primarily 5 thread which get zinfo from zvol_list by taking global lock \"zvol_list_mutex\". This lookup happens via handshake message and take refcount under this lock. These threads are\r\na) mgmt thread b) io receiver thread c) io_ack sender thread d) rebuild scanner thread e) dw_rebuild thead.\r\n2. Once zinfo nuked out of zvol_list, no new thread can take refcount on it.\r\n3. When we destroy/export volumes, it will nuke out zinfo from zvol_list and then wait for refcount to hit to zero. Once it is zero, it will free the zinfo.\r\n\r\nSo far I am convinced with this approach but since it is race, you never know. I request each of you to please provide your valuable input. \r\n\r\nTh\r\n\r\n<!--- Provide a general summary of your changes in the Title above -->\r\n<!--- Explain how the fix was tested -->\r\n"},"comment":{"url":"https://api.github.com/repos/openebs/zfs/issues/comments/402930372","html_url":"https://github.com/openebs/zfs/pull/82#issuecomment-402930372","issue_url":"https://api.github.com/repos/openebs/zfs/issues/82","id":402930372,"node_id":"MDEyOklzc3VlQ29tbWVudDQwMjkzMDM3Mg==","user":{"login":"satbirchhikara","id":34478047,"node_id":"MDQ6VXNlcjM0NDc4MDQ3","avatar_url":"https://avatars2.githubusercontent.com/u/34478047?v=4","gravatar_id":"","url":"https://api.github.com/users/satbirchhikara","html_url":"https://github.com/satbirchhikara","followers_url":"https://api.github.com/users/satbirchhikara/followers","following_url":"https://api.github.com/users/satbirchhikara/following{/other_user}","gists_url":"https://api.github.com/users/satbirchhikara/gists{/gist_id}","starred_url":"https://api.github.com/users/satbirchhikara/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/satbirchhikara/subscriptions","organizations_url":"https://api.github.com/users/satbirchhikara/orgs","repos_url":"https://api.github.com/users/satbirchhikara/repos","events_url":"https://api.github.com/users/satbirchhikara/events{/privacy}","received_events_url":"https://api.github.com/users/satbirchhikara/received_events","type":"User","site_admin":false},"created_at":"2018-07-06T05:32:24Z","updated_at":"2018-07-06T05:32:24Z","author_association":"NONE","body":"Comment related to perfromance and #IOs before and after, we all know that we yet to start perfromance exercise on cStor but coming to this fix, if I am not wrong, I have got comments long back that we are already having so many system calls and refcount increment/decrement of zinfo added four more system call per IO (two system call (lock and unlock) for refcount increment and two system call (lock and unlock) for refcount decrement). \r\n\r\nWe yet to see how much perfromance improvement we will get out of it(may be negligible or may be zero) but to me its more of doing things the way they should be ....if we can live with atomic operation then it is little excess to use mutex for those things. \r\n\r\nHaving said that, I welcome your comments and I am bound to answer your comments/concerns, but in no way I am pushing my changes, it is just a proposal and if we as a team are not comfortable with changes we can drop it for now (or forever).\r\n\r\nI appreciate your efforts to reveiw this proposal and for providing valuable comments.\r\n\r\nThanks & Regards,\r\nSatbir Singh"}}
{"actor":{"display_login":"satbirchhikara"}}