From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Biggers Subject: Re: kernel BUG at drivers/md/persistent-data/dm-space-map-disk.c:178 Date: Tue, 14 Jan 2020 13:52:48 -0800 Message-ID: <20200114215248.GK41220@gmail.com> References: <20200107103546.asf4tmlfdmk6xsub@reti> <20200107104627.plviq37qhok2igt4@reti> <20200107122825.qr7o5d6dpwa6kv62@reti> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20200107122825.qr7o5d6dpwa6kv62@reti> Sender: linux-block-owner@vger.kernel.org To: Mike Snitzer Cc: LVM2 development , markus.schade@gmail.com, ejt@redhat.com, linux-block@vger.kernel.org, dm-devel@redhat.com, joe.thornber@gmail.com, dm-devel@lists.ewheeler.net List-Id: dm-devel.ids On Tue, Jan 07, 2020 at 12:28:25PM +0000, Joe Thornber wrote: > On Tue, Jan 07, 2020 at 10:46:27AM +0000, Joe Thornber wrote: > > I'll get a patch to you later today. > > Eric, > > Patch below. I've run it through a bunch of tests in the dm test suite. But > obviously I have never hit your issue. Will do more testing today. > > - Joe > > > > Author: Joe Thornber > Date: Tue Jan 7 11:58:42 2020 +0000 > > [dm-thin, dm-cache] Fix bug in space-maps. > > The space-maps track the reference counts for disk blocks. There are variants > for tracking metadata blocks, and data blocks. > > We implement transactionality by never touching blocks from the previous > transaction, so we can rollback in the event of a crash. > > When allocating a new block we need to ensure the block is free (has reference > count of 0) in both the current and previous transaction. Prior to this patch we > were doing this by searching for a free block in the previous transaction, and > relying on a 'begin' counter to track where the last allocation in the current > transaction was. This 'begin' field was not being updated in all code paths (eg, > increment of a data block reference count due to breaking sharing of a neighbour > block in the same btree leaf). > > This patch keeps the 'begin' field, but now it's just a hint to speed up the search. > Instead we search the current transaction for a free block, and then double check > it's free in the old transaction. Much simpler. > I happened to notice this patch is on the linux-dm/for-next branch (https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=2137c0dcc04b24efb4c38d4b46b7194575718dd5) and it has: Reported-by: Eric Biggers This is wrong, I didn't report this. I think you meant to put: Reported-by: Eric Wheeler - Eric (the other one) From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Biggers Date: Tue, 14 Jan 2020 13:52:48 -0800 Subject: kernel BUG at drivers/md/persistent-data/dm-space-map-disk.c:178 In-Reply-To: <20200107122825.qr7o5d6dpwa6kv62@reti> References: <20200107103546.asf4tmlfdmk6xsub@reti> <20200107104627.plviq37qhok2igt4@reti> <20200107122825.qr7o5d6dpwa6kv62@reti> Message-ID: <20200114215248.GK41220@gmail.com> List-Id: To: lvm-devel@redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On Tue, Jan 07, 2020 at 12:28:25PM +0000, Joe Thornber wrote: > On Tue, Jan 07, 2020 at 10:46:27AM +0000, Joe Thornber wrote: > > I'll get a patch to you later today. > > Eric, > > Patch below. I've run it through a bunch of tests in the dm test suite. But > obviously I have never hit your issue. Will do more testing today. > > - Joe > > > > Author: Joe Thornber > Date: Tue Jan 7 11:58:42 2020 +0000 > > [dm-thin, dm-cache] Fix bug in space-maps. > > The space-maps track the reference counts for disk blocks. There are variants > for tracking metadata blocks, and data blocks. > > We implement transactionality by never touching blocks from the previous > transaction, so we can rollback in the event of a crash. > > When allocating a new block we need to ensure the block is free (has reference > count of 0) in both the current and previous transaction. Prior to this patch we > were doing this by searching for a free block in the previous transaction, and > relying on a 'begin' counter to track where the last allocation in the current > transaction was. This 'begin' field was not being updated in all code paths (eg, > increment of a data block reference count due to breaking sharing of a neighbour > block in the same btree leaf). > > This patch keeps the 'begin' field, but now it's just a hint to speed up the search. > Instead we search the current transaction for a free block, and then double check > it's free in the old transaction. Much simpler. > I happened to notice this patch is on the linux-dm/for-next branch (https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=2137c0dcc04b24efb4c38d4b46b7194575718dd5) and it has: Reported-by: Eric Biggers This is wrong, I didn't report this. I think you meant to put: Reported-by: Eric Wheeler - Eric (the other one)