Xen devel - Sep 2006 - Broken (and unnecessary) SMP locking in blktap.c

Hi all,

The kernel blktap.c has totally broken locking in req_increase().  It
takes an IRQ lock, then calls a number of non-atomic MM functions, such
as balloon_alloc_empty_page_range() and kzalloc(GFP_KERNEL).  So built
with SMP debugging, this function BUG()s instantly on driver
initialisation.

The locking is actually completely unnecessary too in its current form,
because req_increase() is only ever called right at the start of driver
initialisation:

static int __init blkif_init(void)
{
	int i,ret,blktap_dir;
	tap_blkif_t *info;

	if (!is_running_on_xen())
		return -ENODEV;

	INIT_LIST_HEAD(&pending_free);
        for(i = 0; i < 2; i++) {
		ret = req_increase();
		if (ret)
			break;
	}

is the only caller.  So nothing else in the driver can possibly be
active at this time.

The *only* reason it might be needed is to provide locking for some
hypothetical future use of req_increase() and req_decrease(), which
would need to be locked against each other.  Given that this isn''t done
right now, I suggest we nuke both the locking and the (unused)
req_decrease function; that can easily be resurrected with correct
locking if we want it in the future.

Anyone mind if that function dies?

--Stephen


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

> The *only* reason it might be needed is to provide locking for some
> hypothetical future use of req_increase() and req_decrease(), which
> would need to be locked against each other.  Given that this isn''t
done
> right now, I suggest we nuke both the locking and the (unused)
> req_decrease function; that can easily be resurrected with correct
> locking if we want it in the future.
>
> Anyone mind if that function dies?
Not at all -- req_decrease is generally broken, and removing it and
the locking seem like a very good plan.  thanks.

a.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel