From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from sender163-mail.zoho.com (sender163-mail.zoho.com [74.201.84.163]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.server123.net (Postfix) with ESMTPS for ; Wed, 1 Jun 2016 06:23:30 +0200 (CEST) From: "James Johnston" References: In-Reply-To: Date: Wed, 1 Jun 2016 04:23:17 -0000 Message-ID: <07bf01d1bbbd$53991750$facb45f0$@codenest.com> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Content-Language: en-us Subject: Re: [dm-crypt] [PATCH] dm-log-writes: fix bug with too large bios List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: 'Mikulas Patocka' , "'Alasdair G. Kergon'" , 'Mike Snitzer' , 'Josef Bacik' Cc: dm-devel@redhat.com, 'Eric Wheeler' , 'Tim Small' , 'Kent Overstreet' , linux-bcache@vger.kernel.org, dm-crypt@saout.de, 'Neil Brown' , linux-raid@vger.kernel.org Hi Mikulas, > bio_alloc can allocate a bio with at most BIO_MAX_PAGES (256) vector > entries. However, the incoming bio may have more vector entries if it was > allocated by other means. For example, bcache submits bios with more than > BIO_MAX_PAGES entries. This results in bio_alloc failure. > > To avoid the failure, change the code so that it allocates bio with at > most BIO_MAX_PAGES entries. If the incoming bio has more entries, > bio_add_page will fail and a new bio will be allocated - the code that > handles bio_add_page failure already exists in the dm-log-writes target. > > Also, move atomic_inc(&lc->io_blocks) before bio_alloc to fix a bug that > the target hangs if bio_alloc fails. The error path does put_io_block(lc), > so we must do atomic_inc(&lc->io_blocks) before invoking the error path to > avoid underflow of lc->io_blocks. > > Signed-off-by: Mikulas Patocka > Cc: stable@vger.kernel.org # v4.1+ How does this relate to the previous patch you made to dm-crypt? How best should I test this? It looks like the dm-crypt patch fixed the problem. Should I test by applying this patch ONLY and reverting the dm-crypt patch? (i.e. does this patch also fix the problem.) Or should I just test with both patches applied simultaneously? James From mboxrd@z Thu Jan 1 00:00:00 1970 From: "James Johnston" Subject: RE: [PATCH] dm-log-writes: fix bug with too large bios Date: Wed, 1 Jun 2016 04:23:17 -0000 Message-ID: <07bf01d1bbbd$53991750$facb45f0$@codenest.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Content-Language: en-us Sender: linux-bcache-owner@vger.kernel.org To: 'Mikulas Patocka' , "'Alasdair G. Kergon'" , 'Mike Snitzer' , 'Josef Bacik' Cc: dm-devel@redhat.com, 'Eric Wheeler' , 'Tim Small' , 'Kent Overstreet' , linux-bcache@vger.kernel.org, dm-crypt@saout.de, 'Neil Brown' , linux-raid@vger.kernel.org List-Id: dm-devel.ids Hi Mikulas, > bio_alloc can allocate a bio with at most BIO_MAX_PAGES (256) vector > entries. However, the incoming bio may have more vector entries if it was > allocated by other means. For example, bcache submits bios with more than > BIO_MAX_PAGES entries. This results in bio_alloc failure. > > To avoid the failure, change the code so that it allocates bio with at > most BIO_MAX_PAGES entries. If the incoming bio has more entries, > bio_add_page will fail and a new bio will be allocated - the code that > handles bio_add_page failure already exists in the dm-log-writes target. > > Also, move atomic_inc(&lc->io_blocks) before bio_alloc to fix a bug that > the target hangs if bio_alloc fails. The error path does put_io_block(lc), > so we must do atomic_inc(&lc->io_blocks) before invoking the error path to > avoid underflow of lc->io_blocks. > > Signed-off-by: Mikulas Patocka > Cc: stable@vger.kernel.org # v4.1+ How does this relate to the previous patch you made to dm-crypt? How best should I test this? It looks like the dm-crypt patch fixed the problem. Should I test by applying this patch ONLY and reverting the dm-crypt patch? (i.e. does this patch also fix the problem.) Or should I just test with both patches applied simultaneously? James