We are not talking about memory ordering requirements. There are no memory
ordering requirements introduced in the patch. It really doesn't matter if
the setting of SYSFS_FLAG_REMOVED is seen before or after anything else.
All that matters is that it does get seen, and it will be seen long before
you could possibly describe the situation as 'denial of service'.
However if we do consider memory ordering guarantees we can describe a clear
limit to the possibly delay between SYSFS_FLAG_REMOVED being set, and being
seen. The atomic_inc_not_zero serves as a memory barrier in exactly the same
way that the current code requires atomic_dec_return. So while
if (likely(sd)
&& (sd->s_flags & SYSFS_FLAG_REMOVED) == 0
&& atomic_inc_not_zero(&sd->s_active)) {
could possibly gain a reference even 'after' SYS_FLAG_REMOVED as been set,
a second call to this on the same processor will see SYSFS_FLAG_REMOVED.
So at the absolute most, we could see NCPUS active references gained and
dropped after SYSFS_FLAG_REMOVED was set - a clear limit which is all we need.
I'm still not sure we even need to argue in terms of memory barriers to be
sure the code is correct, but it seems they are sufficient to give a simple
proof.
The sysfs code already has a dependency on implicit memory barriers to ensure
that ->s_sibling points to a completion. My change does not significantly
add to that.
True. However it is raw in the sense that if some other developer wanted
similar functionality in their own code they might copy the low level stuff
as they wouldn't be able to use the sys_* routines directly. Copying
complex code is not a good path to maintainability.
Yes - I think the synergy between locks an refcounts is really interesting...
As you say, simply using a general abstraction which provides these functions
would incur a space cost that you don't want. You avoid this cost by
re-using s_sibling to store a reference to a completion.
Finding a primitive that allows such optimisations is an interesting
challenge.
I really think that atomic_inc_not_zero is sufficient to provide what you
need.
NeilBrown
--
