On Thu, 2019-05-09 at 07:32 +1000, Dave Chinner wrote: > Hmmm, the first wakeup in xsdc is this one, right: > > /* wake up threads waiting in xfs_log_force() */ > wake_up_all(&iclog->ic_force_wait); > > At the end of the iclog iteration loop? That one is under the > ic_loglock - the lock is dropped to run callbacks, then picked up > again once the callbacks are done and before the ic_callback_lock is > dropped (about 10 lines above the wakeup). So unless I'm missing > something (like enough coffee!) that one look fine. You are right. I failed to spot that the spin_unlock unlocks a different lock than the spin_lock above it takes. > ..... > > > I am not sure about xfs_log_force_umount(). Could the unlock > > be moved to after the wake_up_all, or does that create lock > > ordering issues with the xlog_grant_head_wake_all calls? > > Could a simple lock + unlock of log->l_icloglock around the > > wake_up_all do the trick, or is there some other state that > > also needs to stay locked? > > Need to be careful which lock is used with which wait queue :) > > This one is waking the the xc_commit_wait queue (CIL push commit > sequencing wait queue), which is protected by the > log->l_cilp->xc_push_lock. That should nest jsut fine inside any > locks we are holding at this point, so you should just be able to > wrap it. It's not a common code path, though, it'll only hit this > code when the filesystem is already considered to be in an > unrecoverable state. Awesome. -- All Rights Reversed.
Attachment:
signature.asc
Description: This is a digitally signed message part
