{"comment":{"url":"https://api.github.com/repos/linuxbox2/nfs-ganesha/comments/9386647","html_url":"https://github.com/linuxbox2/nfs-ganesha/commit/8303afbfa575eea0f7fa49b467a25ade61e39a64#commitcomment-9386647","id":9386647,"user":{"login":"was4","id":8904538,"avatar_url":"https://avatars.githubusercontent.com/u/8904538?v=3","gravatar_id":"","url":"https://api.github.com/users/was4","html_url":"https://github.com/was4","followers_url":"https://api.github.com/users/was4/followers","following_url":"https://api.github.com/users/was4/following{/other_user}","gists_url":"https://api.github.com/users/was4/gists{/gist_id}","starred_url":"https://api.github.com/users/was4/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/was4/subscriptions","organizations_url":"https://api.github.com/users/was4/orgs","repos_url":"https://api.github.com/users/was4/repos","events_url":"https://api.github.com/users/was4/events{/privacy}","received_events_url":"https://api.github.com/users/was4/received_events","type":"User","site_admin":false},"position":41,"line":258,"path":"src/support/ds.c","commit_id":"8303afbfa575eea0f7fa49b467a25ade61e39a64","created_at":"2015-01-21T22:34:35Z","updated_at":"2015-01-21T22:34:35Z","body":"Correct.  As I wrote: \"barriers\", not \"atomic\".\n\nAnyway, it seems at least we three understand....\n\nMeanwhile, Dan has suggested a compromise for getting these\nout of the fast path -- based on Frank's mention of get_ds\n(which doesn't exist) -- using mostly existing structures.\n\nI've also taken the opportunity to move some things out of the\nlocked sections that could be done outside.\n\nUsing 769 for the export hash and 193 for the DS hash.\n\nComing soon (as soon as pynfs finishes running).\n\n\nOn 1/21/15 5:00 PM, Matt W. Benjamin wrote:\n> Exactly.  I've started to hedge a bit since reading the NetBSD tech-kern thread on correctness in kernel-mode TAILQ, however.\n>\n> Matt\n>\n> ----- \"Charles Hardin\" <ckhardin@exablox.com> wrote:\n>\n>> Locks imply barriers - they have to.\n>>\n>>\n>> It wouldn’t make any sense to have a write reorder past an “unlock” -\n>> else, what would be the point of “locking” a variable to just have the\n>> write go after the xchg on the unlock path.\n>>\n>>\n>> A very “typical” unlock would look something like\n>>\n>>\n>>\n>> void\n>> unlock(locker_t *l)\n>> {\n>> int state;\n>>\n>>\n>> release_barrier(); /* keep all load/store inside the lock region */\n>> state = swap(&l->state, UNLOCKED);\n>>\n>>\n>> /* Flush the swap - but, if the swap is is atomic then this isn’t\n>> really needed */\n>> store_flush();\n>>\n>>\n>> … other stuff like wakups, etc …\n>> }\n>>\n>>\n>>\n>>\n>>\n>>\n>>\n>> On Jan 21, 2015, at 1:23 PM, Matt W. Benjamin < matt@cohortfs.com >\n>> wrote:\n>>\n>> Hi,\n>>\n>> ----- \"William Allen Simpson\" < william.allen.simpson@gmail.com >\n>> wrote:\n>>\n>>\n>>\n>>\n>>\n>>\n>> My suggestion of moving this code into get_ds would then make this\n>> whole\n>>\n>>\n>> business atomic by protecting it with a lock that prevents the ds\n>> from being\n>>\n>>\n>> removed.\n>>\n>> Huh? The only thing that makes something atomic is atomic. Having\n>> an assignment inside a lock doesn't make it atomic.\n>>\n>> It doesn't guarantee the propagation of CPU cached entries, except\n>> for the specific mutex involved. In the default case, it is\n>> supposed to provide a barrier for code order optimization.\n>>\n>>\n>> responding to just this remark, and not the larger thread, this seems\n>> like a red\n>> herring; a pthreads mutex is exactly how we would ensure that a\n>> accesses\n>> to a set of related variables are synchronized, and that is within the\n>> ordinary meaning of \"atomicity.\" if we find a problem with cache\n>> consistency, then explicit barriers by definition will address it.\n>>\n>> Matt\n>>\n>> --\n>> Matt Benjamin\n>> CohortFS, LLC.\n>> 315 West Huron Street, Suite 140A\n>> Ann Arbor, Michigan 48103\n>>\n>> http://cohortfs.com\n>>\n>> tel. 734-761-4689\n>> fax. 734-769-8938\n>> cel. 734-216-5309\n>>\n>> ------------------------------------------------------------------------------\n>> New Year. New Location. New Benefits. New Data Center in Ashburn, VA.\n>> GigeNET is offering a free month of service with a new server in\n>> Ashburn.\n>> Choose from 2 high performing configs, both with 100TB of bandwidth.\n>> Higher redundancy.Lower latency.Increased capacity.Completely\n>> compliant.\n>> http://p.sf.net/sfu/gigenet\n>> _______________________________________________\n>> Nfs-ganesha-devel mailing list\n>> Nfs-ganesha-devel@lists.sourceforge.net\n>> https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel\n>>\n>>\n>> ------------------------------------------------------------------------------\n>> New Year. New Location. New Benefits. New Data Center in Ashburn, VA.\n>> GigeNET is offering a free month of service with a new server in\n>> Ashburn.\n>> Choose from 2 high performing configs, both with 100TB of bandwidth.\n>> Higher redundancy.Lower latency.Increased capacity.Completely\n>> compliant.\n>> http://p.sf.net/sfu/gigenet\n>> _______________________________________________\n>> Nfs-ganesha-devel mailing list\n>> Nfs-ganesha-devel@lists.sourceforge.net\n>> https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel\n>"}}