Re: 2.6.39-rc3, 2.6.39-rc4: XFS lockup - regression since 2.6.38

[Christoph Hellwig]

> 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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/