LU-9679 osc: simplify osc_extent_wait()

Taking a spinlock to check the current value of the state is
unnecessary.  The wake_up() and wait_event() calls have sufficient
barriers to ensure that the value will be seen and the wait will abort
properly.

In most cases, osc_extent_wait() is followed by osc_object_lock()
before any shared data is touched - in those cases there is no need
for osc_extent_wait() to wait for the spinlock to be released.

The one case where osc_object_lock() does not immediately follow is in
osc_cache_truncate_start().  The extra locking was introduced in a
patch which fixed a problem with truncation, so it is likely that this
is the call that was thought to be relevant.

In that case, following osc_extent_wait(), an extent that had been
detached from the per-object list (oe_link linkage) is worked on it
without any locking.

In this case the code is waiting for OES_TRUNC, so any changes that
happen after the osc_extent_state_set(ext, OES_TRUNC) and when the
lock is dropped, might not be seen by the woken code.  The only thing
changed is ->oe_trunc_pending, and the woken code doesn't look at
that.

The only remaining possible need for extra synchronization is if some
other value was changed before the wakeup and is needed after the
wait.  According to memory-barriers.txt, a barrier might be needed to
ensure that is visible.  Such a barrier is most clearly presented by
used smp_store_release() to set the state before wakeup, and
smp_load_acquire() to view it after waiting.

Also use a simple wake_up() instead of wake_up_all() - the latter is
only needed when exclusive waiting is being used.

Linux-Commit: 2a7e8a40cb11 ("lustre: osc: simplify osc_extent_wait()")

Signed-off-by: Mr NeilBrown <neilb@suse.com>
Change-Id: I0945902b55d598af263779787b9641a86e9476c3
Reviewed-on: https://review.whamcloud.com/37598
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Patrick Farrell <farr0186@gmail.com>
Reviewed-by: James Simmons <jsimmons@infradead.org>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/osc/osc_cache.c b/lustre/osc/osc_cache.c
index d3e2378..884d14c 100644 (file)
--- a/lustre/osc/osc_cache.c
+++ b/lustre/osc/osc_cache.c
@@ -314,8 +314,8 @@ static void osc_extent_state_set(struct osc_extent *ext, int state)
        /* LASSERT(sanity_check_nolock(ext) == 0); */
 
        /* TODO: validate the state machine */
-       ext->oe_state = state;
-       wake_up_all(&ext->oe_waitq);
+       smp_store_release(&ext->oe_state, state);
+       wake_up(&ext->oe_waitq);
 }
 
 static struct osc_extent *osc_extent_alloc(struct osc_object *obj)
@@ -893,17 +893,6 @@ int osc_extent_finish(const struct lu_env *env, struct osc_extent *ext,
        RETURN(0);
 }
 
-static int extent_wait_cb(struct osc_extent *ext, enum osc_extent_state state)
-{
-       int ret;
-
-       osc_object_lock(ext->oe_obj);
-       ret = ext->oe_state == state;
-       osc_object_unlock(ext->oe_obj);
-
-       return ret;
-}
-
 /**
  * Wait for the extent's state to become @state.
  */
@@ -932,14 +921,16 @@ static int osc_extent_wait(const struct lu_env *env, struct osc_extent *ext,
                osc_extent_release(env, ext);
 
        /* wait for the extent until its state becomes @state */
-       rc = wait_event_idle_timeout(ext->oe_waitq, extent_wait_cb(ext, state),
+       rc = wait_event_idle_timeout(ext->oe_waitq,
+                                    smp_load_acquire(&ext->oe_state) == state,
                                     cfs_time_seconds(600));
        if (rc == 0) {
                OSC_EXTENT_DUMP(D_ERROR, ext,
                        "%s: wait ext to %u timedout, recovery in progress?\n",
                        cli_name(osc_cli(obj)), state);
 
-               wait_event_idle(ext->oe_waitq, extent_wait_cb(ext, state));
+               wait_event_idle(ext->oe_waitq,
+                               smp_load_acquire(&ext->oe_state) == state);
        }
        if (ext->oe_rc < 0)
                rc = ext->oe_rc;