> The third problem is that updating the push target is not safe on 32
> bit machines. We cannot copy a 64 bit LSN without the possibility of
> corrupting the result when racing with another updating thread. We
> have function to do this update safely without needing to care about
> 32/64 bit issues - xfs_trans_ail_copy_lsn() - so use that when
> updating the AIL push target.
But reading xa_target without xa_lock isn't safe on 32-bit either, is it?
For the first read it can trivially be moved into the critical
section a few lines below, and the second one should probably use
XFS_LSN_CMP.
> @@ -482,19 +481,24 @@ xfs_ail_worker(
> /* assume we have more work to do in a short while */
> tout = 10;
> if (!count) {
> +out_done:
Jumping into conditionals is really ugly. By initializing count a bit
earlier you can just jump in front of the if/else clauses. And while
you're there maybe moving the tout = 10; into an else clause would
also make the code more readable.
an uninitialied used of tout.
> + if (ailp->xa_target == target ||
> + (test_and_set_bit(XFS_AIL_PUSHING_BIT, &ailp->xa_flags)))
no need for braces around the test_and_set_bit call.
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
