From mboxrd@z Thu Jan 1 00:00:00 1970 From: malahal@us.ibm.com Subject: Re: [PATCH] a deadlock bug in the kernel-side device mapper code Date: Mon, 9 Nov 2009 17:50:03 -0800 Message-ID: <20091110015003.GA7104@us.ibm.com> References: <4AF2D176.4010000@actcom.co.il> <20091105142435.GQ13375@agk-dp.fab.redhat.com> <20091109085142.GA4432@linux.vnet.ibm.com> <20091109175730.GA12556@us.ibm.com> <20091110002445.GF17055@agk-dp.fab.redhat.com> Reply-To: device-mapper development Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20091110002445.GF17055@agk-dp.fab.redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com To: dm-devel@redhat.com List-Id: dm-devel.ids Alasdair G Kergon [agk@redhat.com] wrote: > On Mon, Nov 09, 2009 at 09:57:30AM -0800, malahal@us.ibm.com wrote: > > Why do we need dm_get() and dm_put() in dm_copy_name_and_uuid()? > > dm_copy_name_and_uuid() already has access to md and there shouldn't be > > any need to hold a reference. > > As Mike points out, struct dm_uevent holds a reference without > incrementing the ref count. > > dm_path_uevent() already takes a reference - can everything get > simplified if we move this code there (and replace the new mutex with > a spinlock of course)? Then dm_send_uevents won't need to use 'md'. I think the last dm_put() is calling the multipath destructor which eventually calls (through a work struct) dm_send_uevents. At that point, dm is guaranteed to exist but you *must* not place a hold on it. As the current code in dm_copy_name_and_uuid() places a hold on a FREEING md, it will result in a BUG_ON while calling the dm_put in the same function. So, removing the dm_get/dm_put from that function should also solve the BUG_ON issue. I agree, removing the "md" field from the dm_event struct probably simplifies other things. Thanks, Malahal.