[Xen-devel] Broken (and unnecessary) SMP locking in blktap.c

["Stephen C. Tweedie" <sct@xxxxxxxxxx>]

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@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel