Stephen C. Tweedie
2006-Sep-28 16:10 UTC
[Xen-devel] [Patch] Fix SMP debugging assert failures in blktap
blktap is calling non-atomic kernel functions with irqs disabled, which hits various kernel debug traps if those are enabled. The problem is req_increase(), which takes the pending_free_lock. This function is currently only ever called from module initialiation, where it is impossible for it to race against anything else. Its companion, req_decrease(), is not called at all. Fix by removing the offending locking from req_increase() and, while we''re at it, remove req_decrease() entirely. Signed-off-by: Stephen Tweedie <sct@redhat.com> diff -r ed51caee4fe6 -r 94df5bd84195 linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c --- a/linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c Thu Sep 28 15:38:25 2006 +0100 +++ b/linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c Thu Sep 28 17:09:09 2006 +0100 @@ -716,8 +716,6 @@ static int req_increase(void) unsigned long flags; int ret; - spin_lock_irqsave(&pending_free_lock, flags); - ret = -EINVAL; if (mmap_alloc >= MAX_PENDING_REQS || mmap_lock) goto done; @@ -782,8 +780,7 @@ static int req_increase(void) mmap_alloc++; DPRINTK("# MMAPs increased to %d\n",mmap_alloc); - done: - spin_unlock_irqrestore(&pending_free_lock, flags); +done: return ret; } @@ -811,36 +808,6 @@ static void mmap_req_del(int mmap) mmap_lock = 0; DPRINTK("# MMAPs decreased to %d\n",mmap_alloc); mmap_alloc--; -} - -/*N.B. Currently unused - will be accessed via sysfs*/ -static void req_decrease(void) -{ - pending_req_t *req; - int i; - unsigned long flags; - - spin_lock_irqsave(&pending_free_lock, flags); - - DPRINTK("Req decrease called.\n"); - if (mmap_lock || mmap_alloc == 1) - goto done; - - mmap_lock = 1; - mmap_inuse = MAX_PENDING_REQS; - - /*Go through reqs and remove any that aren''t in use*/ - for (i = 0; i < MAX_PENDING_REQS ; i++) { - req = &pending_reqs[mmap_alloc-1][i]; - if (req->inuse == 0) { - list_del(&req->free_list); - mmap_inuse--; - } - } - if (mmap_inuse == 0) mmap_req_del(mmap_alloc-1); - done: - spin_unlock_irqrestore(&pending_free_lock, flags); - return; } static pending_req_t* alloc_req(void) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Andrew Warfield
2006-Sep-28 19:04 UTC
[Xen-devel] Re: [Patch] Fix SMP debugging assert failures in blktap
Applied, thank you. a. On 9/28/06, Stephen C. Tweedie <sct@redhat.com> wrote:> blktap is calling non-atomic kernel functions with irqs disabled, which > hits various kernel debug traps if those are enabled. The problem is > req_increase(), which takes the pending_free_lock. > > This function is currently only ever called from module initialiation, > where it is impossible for it to race against anything else. Its > companion, req_decrease(), is not called at all. > > Fix by removing the offending locking from req_increase() and, while > we''re at it, remove req_decrease() entirely. > > Signed-off-by: Stephen Tweedie <sct@redhat.com> > > diff -r ed51caee4fe6 -r 94df5bd84195 linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c > --- a/linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c Thu Sep 28 15:38:25 2006 +0100 > +++ b/linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c Thu Sep 28 17:09:09 2006 +0100 > @@ -716,8 +716,6 @@ static int req_increase(void) > unsigned long flags; > int ret; > > - spin_lock_irqsave(&pending_free_lock, flags); > - > ret = -EINVAL; > if (mmap_alloc >= MAX_PENDING_REQS || mmap_lock) > goto done; > @@ -782,8 +780,7 @@ static int req_increase(void) > > mmap_alloc++; > DPRINTK("# MMAPs increased to %d\n",mmap_alloc); > - done: > - spin_unlock_irqrestore(&pending_free_lock, flags); > +done: > return ret; > } > > @@ -811,36 +808,6 @@ static void mmap_req_del(int mmap) > mmap_lock = 0; > DPRINTK("# MMAPs decreased to %d\n",mmap_alloc); > mmap_alloc--; > -} > - > -/*N.B. Currently unused - will be accessed via sysfs*/ > -static void req_decrease(void) > -{ > - pending_req_t *req; > - int i; > - unsigned long flags; > - > - spin_lock_irqsave(&pending_free_lock, flags); > - > - DPRINTK("Req decrease called.\n"); > - if (mmap_lock || mmap_alloc == 1) > - goto done; > - > - mmap_lock = 1; > - mmap_inuse = MAX_PENDING_REQS; > - > - /*Go through reqs and remove any that aren''t in use*/ > - for (i = 0; i < MAX_PENDING_REQS ; i++) { > - req = &pending_reqs[mmap_alloc-1][i]; > - if (req->inuse == 0) { > - list_del(&req->free_list); > - mmap_inuse--; > - } > - } > - if (mmap_inuse == 0) mmap_req_del(mmap_alloc-1); > - done: > - spin_unlock_irqrestore(&pending_free_lock, flags); > - return; > } > > static pending_req_t* alloc_req(void) > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel