Linux RAID subsystem development
 help / color / mirror / Atom feed
* Re: [PATCH 0/2] Introduce the request handling for dm-crypt
From: Jan Kara @ 2015-11-12 12:24 UTC (permalink / raw)
  To: Baolin Wang
  Cc: Jan Kara, Christoph Hellwig, axboe, Alasdair G Kergon,
	Mike Snitzer, dm-devel, neilb, tj, jmoyer, keith.busch,
	bart.vanassche, linux-raid, Mark Brown, Arnd Bergmann,
	Garg, Dinesh, LKML
In-Reply-To: <CAMz4kuKmU8E3O6pvE0VXGzJ53uobFzGkNYien0_OXXBssafo3Q@mail.gmail.com>

On Thu 12-11-15 19:46:26, Baolin Wang wrote:
> On 12 November 2015 at 19:06, Jan Kara <jack@suse.cz> wrote:
> > On Thu 12-11-15 17:40:59, Baolin Wang wrote:
> >> On 12 November 2015 at 17:17, Jan Kara <jack@suse.cz> wrote:
> >> > On Thu 12-11-15 10:15:32, Baolin Wang wrote:
> >> >> On 11 November 2015 at 17:48, Christoph Hellwig <hch@infradead.org> wrote:
> >> >> > On Wed, Nov 11, 2015 at 05:31:43PM +0800, Baolin Wang wrote:
> >> >> >> Now the dm-crypt code only implemented the 'based-bio' method to encrypt/
> >> >> >> decrypt block data, which can only hanle one bio at one time. As we know,
> >> >> >> one bio must use the sequential physical address and it also has a limitation
> >> >> >> of length. Thus it may limit the big block encyrtion/decryption when some
> >> >> >> hardware support the big block data encryption.
> >> >> >>
> >> >> >> This patch series introduc the 'based-request' method to handle the data
> >> >> >> encryption/decryption. One request can contain multiple bios, so it can
> >> >> >> handle big block data to improve the efficiency.
> >> >> >
> >> >> > NAK for more request based stacking or DM drivers.  They are a major
> >> >> > pain to deal with, and adding more with different requirements then
> >> >> > dm-multipath is not helping in actually making that one work properly.
> >> >>
> >> >> But now many vendors supply the hardware engine to handle the
> >> >> encyrtion/decryption. The hardware really need a big block to indicate
> >> >> its performance with request based things. Another thing is now the
> >> >> request based things is used by many vendors (Qualcomm, Spreadtrum and
> >> >> so on) to improve their performance and there's a real performance
> >> >> requirement here (I can show the performance result later).
> >> >
> >> > So you've mentioned several times that hardware needs big blocks. How big
> >> > those blocks need to be? Ideally, can you give some numbers on how the
> >> > throughput of the encryption hw grows with the block size?
> >>
> >> It depends on the hardware design. My beaglebone black board's AES
> >> engine can handle 1M at one time which is not big. As I know some
> >> other AES engine can handle 16M data at one time or more.
> >
> > Well, one question is "can handle" and other question is how big gain in
> > throughput it will bring compared to say 1M chunks. I suppose there's some
> > constant overhead to issue a request to the crypto hw and by the time it is
> > encrypting 1M it may be that this overhead is well amortized by the cost of
> > the encryption itself which is in principle linear in the size of the
> > block. That's why I'd like to get idea of the real numbers...
> 
> Please correct me if I misunderstood your point. Let's suppose the AES
> engine can handle 16M at one time. If we give the size of data is less
> than 16M, the engine can handle it at one time. But if the data size
> is 20M (more than 16M), the engine driver will split the data with 16M
> and 4M to deal with. I can not say how many numbers, but I think the
> engine is like to big chunks than small chunks which is the hardware
> engine's advantage.

No, I meant something different. I meant that if HW can encrypt 1M in say
1.05 ms and it can encrypt 16M in 16.05 ms, then although using 16 M blocks
gives you some advantage it becomes diminishingly small.

> >> > You mentioned that you use requests because of size limitations on bios - I
> >> > had a look and current struct bio can easily describe 1MB requests (that's
> >> > assuming 64-bit architecture, 4KB pages) when we have 1 page worth of
> >> > struct bio_vec. Is that not enough?
> >>
> >> Usually one bio does not always use the full 1M, maybe some 1k/2k/8k
> >> or some other small chunks. But request can combine some sequential
> >> small bios to be a big block and it is better than bio at least.
> >
> > As Christoph mentions 4.3 should be better in submitting larger bios. Did
> > you check it?
> 
> I'm sorry I didn't check it. What's the limitation of one bio on 4.3?

On 4.3 it is 1 MB (which should be enough because requests are limited to
512 KB by default anyway). Previously the maximum bio size depended on the
queue parameters such as max number of segments etc.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply

* Re: [PATCH 0/2] Introduce the request handling for dm-crypt
From: Baolin Wang @ 2015-11-12 11:46 UTC (permalink / raw)
  To: Jan Kara
  Cc: Christoph Hellwig, axboe, Alasdair G Kergon, Mike Snitzer,
	dm-devel, neilb, tj, jmoyer, keith.busch, bart.vanassche,
	linux-raid, Mark Brown, Arnd Bergmann, Garg, Dinesh, LKML
In-Reply-To: <20151112110617.GA26662@quack.suse.cz>

On 12 November 2015 at 19:06, Jan Kara <jack@suse.cz> wrote:
> On Thu 12-11-15 17:40:59, Baolin Wang wrote:
>> On 12 November 2015 at 17:17, Jan Kara <jack@suse.cz> wrote:
>> > On Thu 12-11-15 10:15:32, Baolin Wang wrote:
>> >> On 11 November 2015 at 17:48, Christoph Hellwig <hch@infradead.org> wrote:
>> >> > On Wed, Nov 11, 2015 at 05:31:43PM +0800, Baolin Wang wrote:
>> >> >> Now the dm-crypt code only implemented the 'based-bio' method to encrypt/
>> >> >> decrypt block data, which can only hanle one bio at one time. As we know,
>> >> >> one bio must use the sequential physical address and it also has a limitation
>> >> >> of length. Thus it may limit the big block encyrtion/decryption when some
>> >> >> hardware support the big block data encryption.
>> >> >>
>> >> >> This patch series introduc the 'based-request' method to handle the data
>> >> >> encryption/decryption. One request can contain multiple bios, so it can
>> >> >> handle big block data to improve the efficiency.
>> >> >
>> >> > NAK for more request based stacking or DM drivers.  They are a major
>> >> > pain to deal with, and adding more with different requirements then
>> >> > dm-multipath is not helping in actually making that one work properly.
>> >>
>> >> But now many vendors supply the hardware engine to handle the
>> >> encyrtion/decryption. The hardware really need a big block to indicate
>> >> its performance with request based things. Another thing is now the
>> >> request based things is used by many vendors (Qualcomm, Spreadtrum and
>> >> so on) to improve their performance and there's a real performance
>> >> requirement here (I can show the performance result later).
>> >
>> > So you've mentioned several times that hardware needs big blocks. How big
>> > those blocks need to be? Ideally, can you give some numbers on how the
>> > throughput of the encryption hw grows with the block size?
>>
>> It depends on the hardware design. My beaglebone black board's AES
>> engine can handle 1M at one time which is not big. As I know some
>> other AES engine can handle 16M data at one time or more.
>
> Well, one question is "can handle" and other question is how big gain in
> throughput it will bring compared to say 1M chunks. I suppose there's some
> constant overhead to issue a request to the crypto hw and by the time it is
> encrypting 1M it may be that this overhead is well amortized by the cost of
> the encryption itself which is in principle linear in the size of the
> block. That's why I'd like to get idea of the real numbers...

Please correct me if I misunderstood your point. Let's suppose the AES
engine can handle 16M at one time. If we give the size of data is less
than 16M, the engine can handle it at one time. But if the data size
is 20M (more than 16M), the engine driver will split the data with 16M
and 4M to deal with. I can not say how many numbers, but I think the
engine is like to big chunks than small chunks which is the hardware
engine's advantage.

>
>> > You mentioned that you use requests because of size limitations on bios - I
>> > had a look and current struct bio can easily describe 1MB requests (that's
>> > assuming 64-bit architecture, 4KB pages) when we have 1 page worth of
>> > struct bio_vec. Is that not enough?
>>
>> Usually one bio does not always use the full 1M, maybe some 1k/2k/8k
>> or some other small chunks. But request can combine some sequential
>> small bios to be a big block and it is better than bio at least.
>
> As Christoph mentions 4.3 should be better in submitting larger bios. Did
> you check it?

I'm sorry I didn't check it. What's the limitation of one bio on 4.3?
But I think it is better to choose a bigger one ( one request ) for
crypto which is suitable for hardware engine.

>
>                                                                 Honza
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR



-- 
Baolin.wang
Best Regards

^ permalink raw reply

* badblocks seem to be causing problems with raid6 - badblocks list replicating all all drives
From: matt @ 2015-11-12 11:34 UTC (permalink / raw)
  To: linux-raid

Hello,

I posted a while back about getting buffer i/o errors in my dmesg logs 
to my raid array, something along the lines of this:

[158219.456484] EXT4-fs warning (device md4): ext4_end_bio:329: I/O 
error -5 writing to inode 125274714 (offset 176160768 size 8388608 
starting block 4955235712)
[158219.456487] Buffer I/O error on device md4, logical block 4955235584
[158219.456490] Buffer I/O error on device md4, logical block 4955235585
[158219.456491] Buffer I/O error on device md4, logical block 4955235586
[158219.456491] Buffer I/O error on device md4, logical block 4955235587
[158219.456492] Buffer I/O error on device md4, logical block 4955235588
[158219.456493] Buffer I/O error on device md4, logical block 4955235589
[158219.456494] Buffer I/O error on device md4, logical block 4955235590
[158219.456495] Buffer I/O error on device md4, logical block 4955235591
[158219.456496] Buffer I/O error on device md4, logical block 4955235592
[158219.456497] Buffer I/O error on device md4, logical block 4955235593
[158219.456580] EXT4-fs warning (device md4): ext4_end_bio:329: I/O 
error -5 writing to inode 125274714 (offset 176160768 size 8388608 
starting block 4955235456)
[158219.456663] EXT4-fs warning (device md4): ext4_end_bio:329: I/O 
error -5 writing to inode 125274714 (offset 176160768 size 8388608 
starting block 4955235200)
[158219.456747] EXT4-fs warning (device md4): ext4_end_bio:329: I/O 
error -5 writing to inode 125274714 (offset 176160768 size 8388608 
starting block 4955234944)
[158219.456829] EXT4-fs warning (device md4): ext4_end_bio:329: I/O 
error -5 writing to inode 125274714 (offset 176160768 size 8388608 
starting block 4955234688)
[158219.456912] EXT4-fs warning (device md4): ext4_end_bio:329: I/O 
error -5 writing to inode 125274714 (offset 176160768 size 8388608 
starting block 4955234432)
[158469.158278] EXT4-fs warning (device md4): ext4_end_bio:329: I/O 
error -5 writing to inode 123995503 (offset 0 size 8388608 starting 
block 4970080384)
[158469.158281] buffer_io_error: 1526 callbacks suppressed

I am now using the latest mainline kernel, 4.3.0 and I believe something 
is going wrong with the badblocks implementation.

I originally had 3 drives, all with the same badblocks list.  This array 
has been running a while so I have no idea how these 3 discs all ended 
up with the same list of badblocks.

Now, if I remove any drive, which has no badblock entries, and re-add 
it.  Once the sync is complete I end up with another drive with the same 
badblocks list.

At the moment 5 of the drives in the array all have the following 
entries (exactly the same):

Bad-blocks on /dev/sdi1:
           1938038928 for 512 sectors
           1938039440 for 512 sectors
           1938977144 for 512 sectors
           1938977656 for 512 sectors
           3303750816 for 512 sectors
           3303751328 for 512 sectors
           3313648904 for 512 sectors
           3313649416 for 512 sectors
           3313651976 for 512 sectors
           3313652488 for 512 sectors
           3418023432 for 512 sectors
           3418023944 for 512 sectors
           3418024456 for 512 sectors
           3418024968 for 512 sectors
           3418037768 for 512 sectors
           3418038280 for 512 sectors
           3418038792 for 512 sectors
           3418039304 for 512 sectors
           3418112520 for 512 sectors
           3418113032 for 512 sectors
           3418113544 for 512 sectors
           3418114056 for 512 sectors
           3418114568 for 512 sectors
           3418115080 for 512 sectors
           3418124808 for 512 sectors
           3418125320 for 512 sectors
           3418165768 for 512 sectors
           3418166280 for 512 sectors
           3418187272 for 512 sectors
           3418187784 for 512 sectors
           3418213224 for 512 sectors
           3418213736 for 512 sectors
           3418214248 for 512 sectors
           3418214760 for 512 sectors
           3418215272 for 512 sectors
           3418215784 for 512 sectors
           3420607528 for 512 sectors
           3420608040 for 512 sectors
           3420626984 for 512 sectors
           3420627496 for 512 sectors
           3448897824 for 512 sectors
           3448898336 for 512 sectors
           3458897888 for 512 sectors
           3458898400 for 512 sectors
           3519403992 for 512 sectors
           3519404504 for 512 sectors
           3617207456 for 512 sectors
           3617207968 for 512 sectors


How can I clear the badblocks list on all the drives? Something seems 
very wrong and I believe I only actually have 1 faulty disc (I have run 
smartctl long tests on all drives, only 1 failed).

If I can't clear them, how can I get ext4 to recognise the badblocks 
within the array so that it no longer attempts to write to those blocks?

Do the blocks in the list above map to blocks on a the physical 
harddrive, or to blocks on the md device - IE: If that block list was 
passed to ext4 filesystem as bad sectors, would that be the correct 
location on the array or are those the badblocks on one of the 
harddrives in the array.

Thanks

^ permalink raw reply

* Re: [PATCH 0/2] Introduce the request handling for dm-crypt
From: Jan Kara @ 2015-11-12 11:06 UTC (permalink / raw)
  To: Baolin Wang
  Cc: Jan Kara, Christoph Hellwig, axboe, Alasdair G Kergon,
	Mike Snitzer, dm-devel, neilb, tj, jmoyer, keith.busch,
	bart.vanassche, linux-raid, Mark Brown, Arnd Bergmann,
	Garg, Dinesh, LKML
In-Reply-To: <CAMz4kuKBhWcednSauyVhGsSQoNPoLnr4-1RkvJkQvvUR3UpQKg@mail.gmail.com>

On Thu 12-11-15 17:40:59, Baolin Wang wrote:
> On 12 November 2015 at 17:17, Jan Kara <jack@suse.cz> wrote:
> > On Thu 12-11-15 10:15:32, Baolin Wang wrote:
> >> On 11 November 2015 at 17:48, Christoph Hellwig <hch@infradead.org> wrote:
> >> > On Wed, Nov 11, 2015 at 05:31:43PM +0800, Baolin Wang wrote:
> >> >> Now the dm-crypt code only implemented the 'based-bio' method to encrypt/
> >> >> decrypt block data, which can only hanle one bio at one time. As we know,
> >> >> one bio must use the sequential physical address and it also has a limitation
> >> >> of length. Thus it may limit the big block encyrtion/decryption when some
> >> >> hardware support the big block data encryption.
> >> >>
> >> >> This patch series introduc the 'based-request' method to handle the data
> >> >> encryption/decryption. One request can contain multiple bios, so it can
> >> >> handle big block data to improve the efficiency.
> >> >
> >> > NAK for more request based stacking or DM drivers.  They are a major
> >> > pain to deal with, and adding more with different requirements then
> >> > dm-multipath is not helping in actually making that one work properly.
> >>
> >> But now many vendors supply the hardware engine to handle the
> >> encyrtion/decryption. The hardware really need a big block to indicate
> >> its performance with request based things. Another thing is now the
> >> request based things is used by many vendors (Qualcomm, Spreadtrum and
> >> so on) to improve their performance and there's a real performance
> >> requirement here (I can show the performance result later).
> >
> > So you've mentioned several times that hardware needs big blocks. How big
> > those blocks need to be? Ideally, can you give some numbers on how the
> > throughput of the encryption hw grows with the block size?
> 
> It depends on the hardware design. My beaglebone black board's AES
> engine can handle 1M at one time which is not big. As I know some
> other AES engine can handle 16M data at one time or more.

Well, one question is "can handle" and other question is how big gain in
throughput it will bring compared to say 1M chunks. I suppose there's some
constant overhead to issue a request to the crypto hw and by the time it is
encrypting 1M it may be that this overhead is well amortized by the cost of
the encryption itself which is in principle linear in the size of the
block. That's why I'd like to get idea of the real numbers...

> > You mentioned that you use requests because of size limitations on bios - I
> > had a look and current struct bio can easily describe 1MB requests (that's
> > assuming 64-bit architecture, 4KB pages) when we have 1 page worth of
> > struct bio_vec. Is that not enough?
> 
> Usually one bio does not always use the full 1M, maybe some 1k/2k/8k
> or some other small chunks. But request can combine some sequential
> small bios to be a big block and it is better than bio at least.

As Christoph mentions 4.3 should be better in submitting larger bios. Did
you check it?

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply

* Re: [PATCH 0/2] Introduce the request handling for dm-crypt
From: Mark Brown @ 2015-11-12 10:04 UTC (permalink / raw)
  To: Baolin Wang
  Cc: Mike Snitzer, axboe, Alasdair G Kergon, dm-devel, neilb,
	linux-raid, jack, Arnd Bergmann, LKML, keith.busch, jmoyer, tj,
	bart.vanassche, Garg, Dinesh
In-Reply-To: <CAMz4kuKNAkFG4-f6TU1HRxgkAn+YX-D_tTQAWio-A_np3brr+A@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 765 bytes --]

On Thu, Nov 12, 2015 at 04:20:41PM +0800, Baolin Wang wrote:

> 3. perforamence data
> It is just a simple dd test result, and will provide the formal report
> in future. But from the simple test, we can see the improvement.

It's probably also worth pointing out that Qualcomm have been shipping
an out of tree implementation of this as a separate module in their BSP
(originally written by Danesh Garg who's on this thread):

   https://android.googlesource.com/kernel/msm/+/android-msm-dory-3.10-kitkat-wear/drivers/md/dm-req-crypt.c

Android now wants to encrypt phones and tablets by default and have been
seeing substantial performance hits as a result, we can try to get
people to share performance data from productionish systems but it might
be difficult.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply

* Re: [PATCH 0/2] Introduce the request handling for dm-crypt
From: Baolin Wang @ 2015-11-12  9:40 UTC (permalink / raw)
  To: Jan Kara
  Cc: Christoph Hellwig, axboe, Alasdair G Kergon, Mike Snitzer,
	dm-devel, neilb, tj, jmoyer, keith.busch, bart.vanassche,
	linux-raid, Mark Brown, Arnd Bergmann, Garg, Dinesh, LKML
In-Reply-To: <20151112091732.GA23780@quack.suse.cz>

On 12 November 2015 at 17:17, Jan Kara <jack@suse.cz> wrote:
> On Thu 12-11-15 10:15:32, Baolin Wang wrote:
>> On 11 November 2015 at 17:48, Christoph Hellwig <hch@infradead.org> wrote:
>> > On Wed, Nov 11, 2015 at 05:31:43PM +0800, Baolin Wang wrote:
>> >> Now the dm-crypt code only implemented the 'based-bio' method to encrypt/
>> >> decrypt block data, which can only hanle one bio at one time. As we know,
>> >> one bio must use the sequential physical address and it also has a limitation
>> >> of length. Thus it may limit the big block encyrtion/decryption when some
>> >> hardware support the big block data encryption.
>> >>
>> >> This patch series introduc the 'based-request' method to handle the data
>> >> encryption/decryption. One request can contain multiple bios, so it can
>> >> handle big block data to improve the efficiency.
>> >
>> > NAK for more request based stacking or DM drivers.  They are a major
>> > pain to deal with, and adding more with different requirements then
>> > dm-multipath is not helping in actually making that one work properly.
>>
>> But now many vendors supply the hardware engine to handle the
>> encyrtion/decryption. The hardware really need a big block to indicate
>> its performance with request based things. Another thing is now the
>> request based things is used by many vendors (Qualcomm, Spreadtrum and
>> so on) to improve their performance and there's a real performance
>> requirement here (I can show the performance result later).
>
> So you've mentioned several times that hardware needs big blocks. How big
> those blocks need to be? Ideally, can you give some numbers on how the
> throughput of the encryption hw grows with the block size?

It depends on the hardware design. My beaglebone black board's AES
engine can handle 1M at one time which is not big. As I know some
other AES engine can handle 16M data at one time or more.

>
> Because as Mike had said there are downsides to having request based
> dm-crypt as well. E.g. if you want to have encrypted raid5 volume then
> you'd rather want to put encryption on top of raid5 (easier management,
> larger sequential blocks to encrypt, ...) but you cannot do that when
> dm-crypt would be request based. So modifying bio-based dm-crypt to form
> larger chunks for encryption HW would be superior in this regard.
>

Make sense.

> You mentioned that you use requests because of size limitations on bios - I
> had a look and current struct bio can easily describe 1MB requests (that's
> assuming 64-bit architecture, 4KB pages) when we have 1 page worth of
> struct bio_vec. Is that not enough?

Usually one bio does not always use the full 1M, maybe some 1k/2k/8k
or some other small chunks. But request can combine some sequential
small bios to be a big block and it is better than bio at least.

>
>                                                                 Honza
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR



-- 
Baolin.wang
Best Regards

^ permalink raw reply

* Re: [PATCH 0/2] Introduce the request handling for dm-crypt
From: Jan Kara @ 2015-11-12  9:17 UTC (permalink / raw)
  To: Baolin Wang
  Cc: Christoph Hellwig, axboe, agk, snitzer, dm-devel, neilb, jack, tj,
	jmoyer, keith.busch, bart.vanassche, linux-raid, Mark Brown,
	Arnd Bergmann, Garg, Dinesh, LKML
In-Reply-To: <CAMz4ku+VF7-kLyE1fK45_MZ=-Tw3PqpySq3Cu86q+rZX4wRemw@mail.gmail.com>

On Thu 12-11-15 10:15:32, Baolin Wang wrote:
> On 11 November 2015 at 17:48, Christoph Hellwig <hch@infradead.org> wrote:
> > On Wed, Nov 11, 2015 at 05:31:43PM +0800, Baolin Wang wrote:
> >> Now the dm-crypt code only implemented the 'based-bio' method to encrypt/
> >> decrypt block data, which can only hanle one bio at one time. As we know,
> >> one bio must use the sequential physical address and it also has a limitation
> >> of length. Thus it may limit the big block encyrtion/decryption when some
> >> hardware support the big block data encryption.
> >>
> >> This patch series introduc the 'based-request' method to handle the data
> >> encryption/decryption. One request can contain multiple bios, so it can
> >> handle big block data to improve the efficiency.
> >
> > NAK for more request based stacking or DM drivers.  They are a major
> > pain to deal with, and adding more with different requirements then
> > dm-multipath is not helping in actually making that one work properly.
> 
> But now many vendors supply the hardware engine to handle the
> encyrtion/decryption. The hardware really need a big block to indicate
> its performance with request based things. Another thing is now the
> request based things is used by many vendors (Qualcomm, Spreadtrum and
> so on) to improve their performance and there's a real performance
> requirement here (I can show the performance result later).

So you've mentioned several times that hardware needs big blocks. How big
those blocks need to be? Ideally, can you give some numbers on how the
throughput of the encryption hw grows with the block size?

Because as Mike had said there are downsides to having request based
dm-crypt as well. E.g. if you want to have encrypted raid5 volume then
you'd rather want to put encryption on top of raid5 (easier management,
larger sequential blocks to encrypt, ...) but you cannot do that when
dm-crypt would be request based. So modifying bio-based dm-crypt to form
larger chunks for encryption HW would be superior in this regard.

You mentioned that you use requests because of size limitations on bios - I
had a look and current struct bio can easily describe 1MB requests (that's
assuming 64-bit architecture, 4KB pages) when we have 1 page worth of
struct bio_vec. Is that not enough?

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply

* Re: [dm-devel] [PATCH 0/2] Introduce the request handling for dm-crypt
From: Christoph Hellwig @ 2015-11-12  9:06 UTC (permalink / raw)
  To: device-mapper development
  Cc: Mike Snitzer, axboe, keith.busch, jack, Arnd Bergmann, neilb,
	LKML, linux-raid, Mark Brown, Garg, Dinesh, tj, bart.vanassche,
	jmoyer, agk
In-Reply-To: <CAMz4kuLHB=tAgJu1T433FTF-S08HcGCP16-urcZA4fp8dtWLMQ@mail.gmail.com>

On Thu, Nov 12, 2015 at 10:36:34AM +0800, Baolin Wang wrote:
> That's right. But we'll not introduce the duality things, cause we
> will remove the bio based things in dm-crypt if the request based
> things are accepted.

No, you will NOT remote the bio based path.  That would break all kinds
of perfectly valid setups.

^ permalink raw reply

* Re: [dm-devel] [PATCH 0/2] Introduce the request handling for dm-crypt
From: Christoph Hellwig @ 2015-11-12  9:05 UTC (permalink / raw)
  To: device-mapper development
  Cc: Baolin Wang, axboe, keith.busch, jack, arnd, neilb, linux-kernel,
	linux-raid, broonie, dineshg, tj, bart.vanassche, jmoyer, agk
In-Reply-To: <20151111181813.GD12236@redhat.com>

On Wed, Nov 11, 2015 at 01:18:13PM -0500, Mike Snitzer wrote:
> But I'm trying to keep an open mind... show me data that real hardware
> _really_ benefits and we'll go from there.  Again, it needs to be "OMG,
> this is amazing!" level performance to warrant any further serious
> consideration.

Also the numbers should be on 4.3+ with our arbitrarily sizeds bios,
and a file system that isn't stupid and actually submits bios (xfs,
btrfs for example).

^ permalink raw reply

* Re: [PATCH 0/2] Introduce the request handling for dm-crypt
From: Baolin Wang @ 2015-11-12  8:20 UTC (permalink / raw)
  To: Mike Snitzer
  Cc: axboe, Alasdair G Kergon, dm-devel, neilb, linux-raid, jack,
	Arnd Bergmann, LKML, keith.busch, jmoyer, Mark Brown, tj,
	bart.vanassche, Garg, Dinesh
In-Reply-To: <20151111181813.GD12236@redhat.com>

On 12 November 2015 at 02:18, Mike Snitzer <snitzer@redhat.com> wrote:
> On Wed, Nov 11 2015 at  4:31am -0500,
> Baolin Wang <baolin.wang@linaro.org> wrote:
>
>> Now the dm-crypt code only implemented the 'based-bio' method to encrypt/
>> decrypt block data, which can only hanle one bio at one time. As we know,
>> one bio must use the sequential physical address and it also has a limitation
>> of length. Thus it may limit the big block encyrtion/decryption when some
>> hardware support the big block data encryption.
>>
>> This patch series introduc the 'based-request' method to handle the data
>> encryption/decryption. One request can contain multiple bios, so it can
>> handle big block data to improve the efficiency.
>
> The duality of bio-based vs request-based code paths in DM core frankly
> sucks.  So the prospect of polluting dm-crypt with a similar duality is
> really _not_ interesting.
>
> Request-based DM requires more memory reserves per device than bio-based
> DM.  Also, you cannot stack request-based DM ontop of bio-based devices
> (be them DM, MD, etc) so request-based DM's underlying storage stack
> gets a lot less interesting with this change.
>
> That said, it could be that the benefits of supporting both bio-based
> and request-based DM in dm-crypt outweigh any overhead/limitations.  But
> you haven't given any performance data to justify this patchset.
>
> There needs to be a _really_ compelling benefit to do this.
>
> Also, FYI, having a big CONFIG knob to switch all of dm-crypt from
> bio-based to request-based is _not_ acceptable.  Both modes would need
> to be supported in parallel.  Could easily be that not all devices in a
> system will benefit from being request-based.
>
> Regardless, the risk of this change causing request-based DM to become
> more brittle than it already is concerns me.
>
> But I'm trying to keep an open mind... show me data that real hardware
> _really_ benefits and we'll go from there.  Again, it needs to be "OMG,
> this is amazing!" level performance to warrant any further serious
> consideration.

Thanks for your suggestion. But let me explain it again. Now for many
vendors, they supply the encryption hardware (such as AES engine) to
accelerate the encyrtion/decryption speed with handling a big block at
one time. So if we want the hardware engine can play the best
performance, the size of block handled at one time need to be
expanded.

But it can only handle one bio at one time for bio based dm-crypt, one
bio has a size limitation and one bio's size can't make the hardware
engine reach its best performance. So we want to introduce the request
based dm-crypt. For request based things, some sequential bios can
merged into one request to expand the IO size to be a big block
handled by hardware engine at one time. With the hardware
acceleration, it can improve the encryption/decryption speed.

I think 3 questions need to be clarified.

1. Are there ways of enhancing the dm-crypt bio-based target to overcome this?
The focus is the size limitation of one bio, its size can not meet the
hardware requirement. But one request can have a big block size with
merging multiple bios. So I think the request is the best choice.

2. Would any sort of bio aggregation mechanism help?
The request can combined sequential bios by block layer automatically.
But for bio aggregation, I think it will be similar to that, why do we
need recomplement it again?

3. perforamence data
It is just a simple dd test result, and will provide the formal report
in future. But from the simple test, we can see the improvement.
Hardware environment:
Board: beaglebone black
processor: AM335x 1GHz ARM Cortex-A8
RAM: 512M
Cipher: cbc(aes) with AES hardware engine

(1) bio based dm-crypt with hardware accelarate:
read 64M command: dd if=/dev/dm-0 of=/dev/null bs=512k count=128 iflag=direct
67108864 bytes (67 MB) copied, 11.6592 s, 5.8 MB/s
67108864 bytes (67 MB) copied, 11.6391 s, 5.8 MB/s
67108864 bytes (67 MB) copied, 11.6296 s, 5.8 MB/s

(2) request based dm-crypt with hardware accelarate
read 64M command: dd if=/dev/dm-0 of=/dev/null bs=512k count=128 iflag=direct
67108864 bytes (67 MB) copied, 5.16586 s, 13.0 MB/s
67108864 bytes (67 MB) copied, 5.19338 s, 12.9 MB/s
67108864 bytes (67 MB) copied, 5.19169 s, 12.9 MB/s

(3) bio based dm-crypt with hardware accelarate
write 64M command: dd if=/dev/zero of=/dev/dm-0 bs=512k count=128 iflag=direct
67108864 bytes (67 MB) copied, 13.6852 s, 4.9 MB/s
67108864 bytes (67 MB) copied, 14.0873 s, 4.8 MB/s
67108864 bytes (67 MB) copied, 13.6649 s, 4.9 MB/s

(4) request based dm-crypt with hardware accelarate
write 64M command: dd if=/dev/zero of=/dev/dm-0 bs=512k count=128 iflag=direct
67108864 bytes (67 MB) copied, 7.27832 s, 9.2 MB/s
67108864 bytes (67 MB) copied, 7.29051 s, 9.2 MB/s
67108864 bytes (67 MB) copied, 7.28318 s, 9.2 MB/s

From the simple result, we can see it at least has a double
improvement of the encryption performance.



-- 
Baolin.wang
Best Regards

^ permalink raw reply

* Re: Raid server Motherboard recommendation.
From: Roman Mamedov @ 2015-11-12  7:29 UTC (permalink / raw)
  To: Ram Ramesh; +Cc: Linux Raid
In-Reply-To: <5643FD4A.4070709@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 373 bytes --]

On Wed, 11 Nov 2015 20:45:30 -0600
Ram Ramesh <rramesh2400@gmail.com> wrote:

> ports are on ASMedia ASM 1061 chipset which apparently is not supported 
> on linux per what I can find on various comments/information from the net

What.

http://superuser.com/questions/363144/is-asmedia-asm1061-sata3-controller-supported-under-linux


-- 
With respect,
Roman

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: Raid server Motherboard recommendation.
From: Ram Ramesh @ 2015-11-12  5:04 UTC (permalink / raw)
  To: Weedy; +Cc: Linux RAID
In-Reply-To: <CAFE24U24eZx0v_HO_jHsxn2JiaGdTrcTB3RUCQ0wUcxHsDRYvA@mail.gmail.com>

On 11/11/2015 09:00 PM, Weedy wrote:
>
>
> On 11 Nov 2015 21:46, "Ram Ramesh" <rramesh2400@gmail.com 
> <mailto:rramesh2400@gmail.com>> wrote:
> >
> > All others seem to be X99 or other server chipsets which I am not 
> fond of. I like desktop boards as the server also doubles as mythtv 
> backend/frontend.
>
> ASrock has an atom board that takes ECC and has 10 or 12 sata ports. 
> They should all be Intel ports.
>
Thanks. I guess you mean this?
> ASRock C2750D4I Mini ITX Server Motherboard FCBGA1283 DDR3 1600/1333
It does look good for what it offers. It is a server board and 6 ports 
are on Marvel controller. Further this is mini ITX and possibly no video 
card. Limited for mythtv purpose. Not to mention $400 :-)

Ramesh


^ permalink raw reply

* Re: [PATCH 1/2] block: Introduce BIO_ENDIO_FREE for bio flags
From: Baolin Wang @ 2015-11-12  4:05 UTC (permalink / raw)
  To: Mike Snitzer
  Cc: axboe, Alasdair G Kergon, dm-devel, neilb, linux-raid, jack,
	Arnd Bergmann, LKML, keith.busch, jmoyer, Mark Brown, tj,
	bart.vanassche, Garg, Dinesh
In-Reply-To: <20151111175411.GC12236@redhat.com>

On 12 November 2015 at 01:54, Mike Snitzer <snitzer@redhat.com> wrote:
> On Wed, Nov 11 2015 at  4:31am -0500,
> Baolin Wang <baolin.wang@linaro.org> wrote:
>
>> When we use dm-crypt to decrypt block data, it will decrypt the block data
>> in endio() when one IO is completed. In this situation we don't want the
>> cloned bios is freed before calling the endio().
>>
>> Thus introduce 'BIO_ENDIO_FREE' flag to support the request handling for dm-crypt,
>> this flag will ensure that blk layer does not complete the cloned bios before
>> completing the request. When the crypt endio is called, post-processsing is
>> done and then the dm layer will complete the bios (clones) and free them.
>
> Not following why request-based DM's partial completion handling
> (drivers/md/dm.c:end_clone_bio) isn't a sufficient hook -- no need to
> add block complexity.
>

Sorry for lacking of more explanation for that. The dm-crypt will
decrypt block data in the end_io() callback function when one request
is completed, so we don't want the bios of this request is freed when
calling the end_io() callback. Thus we introduce a flag to indicate
these type bios of this request will be freed at dm layer not in block
layer.

> But that aside, I'm not liking the idea of a request-based dm-crypt.
>
>> diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
>> index 76d23fa..f636c50 100644
>> --- a/include/linux/device-mapper.h
>> +++ b/include/linux/device-mapper.h
>> @@ -407,6 +407,11 @@ union map_info *dm_get_rq_mapinfo(struct request *rq);
>>
>>  struct queue_limits *dm_get_queue_limits(struct mapped_device *md);
>>
>> +void dm_end_request(struct request *clone, int error);
>> +void dm_kill_unmapped_request(struct request *rq, int error);
>> +void dm_dispatch_clone_request(struct request *clone, struct request *rq);
>> +struct request *dm_get_orig_rq(struct request *clone);
>> +
>>  /*
>>   * Geometry functions.
>>   */
>
> I have no interest in seeing any request-based DM interfaces exported.

OK.


-- 
Baolin.wang
Best Regards

^ permalink raw reply

* Raid server Motherboard recommendation.
From: Ram Ramesh @ 2015-11-12  2:45 UTC (permalink / raw)
  To: Linux Raid

All,

  It is time for my server to be upgraded and I am looking for advice. I 
usually build my machine so, I am looking primarily for a motherboard 
that has several sata ports that is  compatible with linux.

I looked at Asrock extreme6, but apparently it has 4 of its 10 sata 
ports are on ASMedia ASM 1061 chipset which apparently is not supported 
on linux per what I can find on various comments/information from the net

All others seem to be X99 or other server chipsets which I am not fond 
of. I like desktop boards as the server also doubles as mythtv 
backend/frontend.

Let me know if this is the wrong thread and I should ask some where else.

Regards
Ramesh


^ permalink raw reply

* Re: [PATCH 0/2] Introduce the request handling for dm-crypt
From: Baolin Wang @ 2015-11-12  2:36 UTC (permalink / raw)
  To: Mike Snitzer
  Cc: axboe, agk, dm-devel, neilb, linux-raid, jack, Arnd Bergmann,
	LKML, keith.busch, jmoyer, Mark Brown, tj, bart.vanassche,
	Garg, Dinesh
In-Reply-To: <20151111181813.GD12236@redhat.com>

On 12 November 2015 at 02:18, Mike Snitzer <snitzer@redhat.com> wrote:
> On Wed, Nov 11 2015 at  4:31am -0500,
> Baolin Wang <baolin.wang@linaro.org> wrote:
>
>> Now the dm-crypt code only implemented the 'based-bio' method to encrypt/
>> decrypt block data, which can only hanle one bio at one time. As we know,
>> one bio must use the sequential physical address and it also has a limitation
>> of length. Thus it may limit the big block encyrtion/decryption when some
>> hardware support the big block data encryption.
>>
>> This patch series introduc the 'based-request' method to handle the data
>> encryption/decryption. One request can contain multiple bios, so it can
>> handle big block data to improve the efficiency.
>
> The duality of bio-based vs request-based code paths in DM core frankly
> sucks.  So the prospect of polluting dm-crypt with a similar duality is
> really _not_ interesting.
>

That's right. But we'll not introduce the duality things, cause we
will remove the bio based things in dm-crypt if the request based
things are accepted.

> Request-based DM requires more memory reserves per device than bio-based
> DM.  Also, you cannot stack request-based DM ontop of bio-based devices
> (be them DM, MD, etc) so request-based DM's underlying storage stack
> gets a lot less interesting with this change.
>

Yes, the request based requires more memory than bio based, but it is
not too much. And the request based has a big performance improvement.

> That said, it could be that the benefits of supporting both bio-based
> and request-based DM in dm-crypt outweigh any overhead/limitations.  But
> you haven't given any performance data to justify this patchset.
>

Like I said above, we plan to remove the bio based things which are
not good support for hardware engine encryption. And I'll show you the
performance data to prove the request things have a good performance.

> There needs to be a _really_ compelling benefit to do this.
>
> Also, FYI, having a big CONFIG knob to switch all of dm-crypt from
> bio-based to request-based is _not_ acceptable.  Both modes would need
> to be supported in parallel.  Could easily be that not all devices in a
> system will benefit from being request-based.
>

OK. The CONFIG is not suitable here. I'll remove the CONFIG with just
enable the request based things.

> Regardless, the risk of this change causing request-based DM to become
> more brittle than it already is concerns me.
>
> But I'm trying to keep an open mind... show me data that real hardware
> _really_ benefits and we'll go from there.  Again, it needs to be "OMG,
> this is amazing!" level performance to warrant any further serious
> consideration.

OK. I'll show you the performance data. Thanks for your comments.



-- 
Baolin.wang
Best Regards

^ permalink raw reply

* Re: [PATCH 0/2] Introduce the request handling for dm-crypt
From: Baolin Wang @ 2015-11-12  2:15 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: axboe, agk, snitzer, dm-devel, neilb, jack, tj, jmoyer,
	keith.busch, bart.vanassche, linux-raid, Mark Brown,
	Arnd Bergmann, Garg, Dinesh, LKML
In-Reply-To: <20151111094811.GA3641@infradead.org>

On 11 November 2015 at 17:48, Christoph Hellwig <hch@infradead.org> wrote:
> On Wed, Nov 11, 2015 at 05:31:43PM +0800, Baolin Wang wrote:
>> Now the dm-crypt code only implemented the 'based-bio' method to encrypt/
>> decrypt block data, which can only hanle one bio at one time. As we know,
>> one bio must use the sequential physical address and it also has a limitation
>> of length. Thus it may limit the big block encyrtion/decryption when some
>> hardware support the big block data encryption.
>>
>> This patch series introduc the 'based-request' method to handle the data
>> encryption/decryption. One request can contain multiple bios, so it can
>> handle big block data to improve the efficiency.
>
> NAK for more request based stacking or DM drivers.  They are a major
> pain to deal with, and adding more with different requirements then
> dm-multipath is not helping in actually making that one work properly.

But now many vendors supply the hardware engine to handle the
encyrtion/decryption. The hardware really need a big block to indicate
its performance with request based things. Another thing is now the
request based things is used by many vendors (Qualcomm, Spreadtrum and
so on) to improve their performance and there's a real performance
requirement here (I can show the performance result later).

I don't think you will worry if any one can work properly, we will
remove the bio based things in future if the request things are
accepted and proved effectively. Thanks for your comment.

-- 
Baolin.wang
Best Regards

^ permalink raw reply

* Re: [PATCH 0/2] Introduce the request handling for dm-crypt
From: Mike Snitzer @ 2015-11-11 18:18 UTC (permalink / raw)
  To: Baolin Wang
  Cc: axboe, agk, dm-devel, neilb, linux-raid, jack, arnd, linux-kernel,
	keith.busch, jmoyer, broonie, tj, bart.vanassche, dineshg
In-Reply-To: <cover.1447233227.git.baolin.wang@linaro.org>

On Wed, Nov 11 2015 at  4:31am -0500,
Baolin Wang <baolin.wang@linaro.org> wrote:

> Now the dm-crypt code only implemented the 'based-bio' method to encrypt/
> decrypt block data, which can only hanle one bio at one time. As we know,
> one bio must use the sequential physical address and it also has a limitation
> of length. Thus it may limit the big block encyrtion/decryption when some
> hardware support the big block data encryption.
> 
> This patch series introduc the 'based-request' method to handle the data
> encryption/decryption. One request can contain multiple bios, so it can
> handle big block data to improve the efficiency.

The duality of bio-based vs request-based code paths in DM core frankly
sucks.  So the prospect of polluting dm-crypt with a similar duality is
really _not_ interesting.

Request-based DM requires more memory reserves per device than bio-based
DM.  Also, you cannot stack request-based DM ontop of bio-based devices
(be them DM, MD, etc) so request-based DM's underlying storage stack
gets a lot less interesting with this change.

That said, it could be that the benefits of supporting both bio-based
and request-based DM in dm-crypt outweigh any overhead/limitations.  But
you haven't given any performance data to justify this patchset.

There needs to be a _really_ compelling benefit to do this.

Also, FYI, having a big CONFIG knob to switch all of dm-crypt from
bio-based to request-based is _not_ acceptable.  Both modes would need
to be supported in parallel.  Could easily be that not all devices in a
system will benefit from being request-based.

Regardless, the risk of this change causing request-based DM to become
more brittle than it already is concerns me.

But I'm trying to keep an open mind... show me data that real hardware
_really_ benefits and we'll go from there.  Again, it needs to be "OMG,
this is amazing!" level performance to warrant any further serious
consideration.

^ permalink raw reply

* Re: [PATCH 1/2] block: Introduce BIO_ENDIO_FREE for bio flags
From: Mike Snitzer @ 2015-11-11 17:54 UTC (permalink / raw)
  To: Baolin Wang
  Cc: axboe, agk, dm-devel, neilb, linux-raid, jack, arnd, linux-kernel,
	keith.busch, jmoyer, broonie, tj, bart.vanassche, dineshg
In-Reply-To: <2bd3cc769530061501af545c7949dcc832263930.1447233227.git.baolin.wang@linaro.org>

On Wed, Nov 11 2015 at  4:31am -0500,
Baolin Wang <baolin.wang@linaro.org> wrote:

> When we use dm-crypt to decrypt block data, it will decrypt the block data
> in endio() when one IO is completed. In this situation we don't want the
> cloned bios is freed before calling the endio().
> 
> Thus introduce 'BIO_ENDIO_FREE' flag to support the request handling for dm-crypt,
> this flag will ensure that blk layer does not complete the cloned bios before
> completing the request. When the crypt endio is called, post-processsing is
> done and then the dm layer will complete the bios (clones) and free them.

Not following why request-based DM's partial completion handling
(drivers/md/dm.c:end_clone_bio) isn't a sufficient hook -- no need to
add block complexity.

But that aside, I'm not liking the idea of a request-based dm-crypt.

> diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
> index 76d23fa..f636c50 100644
> --- a/include/linux/device-mapper.h
> +++ b/include/linux/device-mapper.h
> @@ -407,6 +407,11 @@ union map_info *dm_get_rq_mapinfo(struct request *rq);
>  
>  struct queue_limits *dm_get_queue_limits(struct mapped_device *md);
>  
> +void dm_end_request(struct request *clone, int error);
> +void dm_kill_unmapped_request(struct request *rq, int error);
> +void dm_dispatch_clone_request(struct request *clone, struct request *rq);
> +struct request *dm_get_orig_rq(struct request *clone);
> +
>  /*
>   * Geometry functions.
>   */

I have no interest in seeing any request-based DM interfaces exported.

^ permalink raw reply

* Re: broken raid level 5 array caused by user error
From: Mathias Mueller @ 2015-11-11 17:53 UTC (permalink / raw)
  To: Phil Turmel; +Cc: Linux raid
In-Reply-To: <56429326.5030405@turmel.org>

Hi Phil,

> So, I fully expected one of those first four combinations to work.  I
> don't know why they didn't.
> 
> The fsck version dates to 2013, so it might be something related to
> that.  My next step would be to try all of this after booting into a
> modern environment, like the rescue cd from sysrescuecd.org.  Recheck
> serial numbers and device names again, if you do that.

I tried the first four combinations with latest sysrescuecd, but the 
results are the same.

Mathias

^ permalink raw reply

* Re: [RFC PATCH 00/32] separate operations from flags in the bio/request structs
From: Mike Snitzer @ 2015-11-11 17:37 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Mike Christie, linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA, device-mapper development,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	drbd-dev-cunTk1MwBs8qoQakbn7OcQ
In-Reply-To: <20151111112846.GA358-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>

On Wed, Nov 11 2015 at  6:28am -0500,
Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:

> On Wed, Nov 11, 2015 at 01:53:24AM -0600, Mike Christie wrote:
> > We no longer have the bvec merge functions so the original reason given
> > in the thread/patch Bart referenced is no longer valid.
> > 
> > Offlist it was suggested that dropping the argument from submit_bio
> > might still improve performance, but I modified xfs and dm and did some
> > testing and did not see anything.
> > 
> > So the change is not needed, and it would only be done because people
> > feel it would improve the interface.
> 
> I would defintively prefer the changed interface, and to me it also
> looks like it would make your overall patch simpler.  If you disagree
> feel free to keep it as-is for now and I'll do another pass later.

The less churn the better.  But honestly, not quite following the logic
on why all this flag-day churn is needed.  BUT if it needs to happen,
because we'll soon hit a wall on supported operations via REQ_*, now is
as good a time as any -- but best to limit the change to the flags IMHO.

^ permalink raw reply

* Re: [dm-devel] [RFC PATCH 00/32] separate operations from flags in the bio/request structs
From: Christoph Hellwig @ 2015-11-11 11:28 UTC (permalink / raw)
  To: Mike Christie
  Cc: linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA, device-mapper development,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	drbd-dev-cunTk1MwBs8qoQakbn7OcQ
In-Reply-To: <5642F3F4.2060007-hcNo3dDEHLuVc3sceRu5cw@public.gmane.org>

On Wed, Nov 11, 2015 at 01:53:24AM -0600, Mike Christie wrote:
> We no longer have the bvec merge functions so the original reason given
> in the thread/patch Bart referenced is no longer valid.
> 
> Offlist it was suggested that dropping the argument from submit_bio
> might still improve performance, but I modified xfs and dm and did some
> testing and did not see anything.
> 
> So the change is not needed, and it would only be done because people
> feel it would improve the interface.

I would defintively prefer the changed interface, and to me it also
looks like it would make your overall patch simpler.  If you disagree
feel free to keep it as-is for now and I'll do another pass later.

^ permalink raw reply

* Re: Buffer I/O errors & Kernel OOPS with RAID6
From: matt @ 2015-11-11 10:59 UTC (permalink / raw)
  To: linux-raid
In-Reply-To: <20151109173507.GA58083@kernel.org>

Hello,

I have now upgraded to 4.3.0

As a test, I have failed and removed a disc with no badblocks, and then 
added it back in, and now the badblock list is the same as my other 
drives with badblocks.

This list of badblocks exists on four of the harddrives now:

Bad-blocks on /dev/sdp1:
           1938038928 for 512 sectors
           1938039440 for 512 sectors
           1938977144 for 512 sectors
           1938977656 for 512 sectors
           3303750816 for 512 sectors
           3303751328 for 512 sectors
           3313648904 for 512 sectors
           3313649416 for 512 sectors
           3313651976 for 512 sectors
           3313652488 for 512 sectors
           3418023432 for 512 sectors
           3418023944 for 512 sectors
           3418024456 for 512 sectors
           3418024968 for 512 sectors
           3418037768 for 512 sectors
           3418038280 for 512 sectors
           3418038792 for 512 sectors
           3418039304 for 512 sectors
           3418112520 for 512 sectors
           3418113032 for 512 sectors
           3418113544 for 512 sectors
           3418114056 for 512 sectors
           3418114568 for 512 sectors
           3418115080 for 512 sectors
           3418124808 for 512 sectors
           3418125320 for 512 sectors
           3418165768 for 512 sectors
           3418166280 for 512 sectors
           3418187272 for 512 sectors
           3418187784 for 512 sectors
           3418213224 for 512 sectors
           3418213736 for 512 sectors
           3418214248 for 512 sectors
           3418214760 for 512 sectors
           3418215272 for 512 sectors
           3418215784 for 512 sectors
           3420607528 for 512 sectors
           3420608040 for 512 sectors
           3420626984 for 512 sectors
           3420627496 for 512 sectors
           3448897824 for 512 sectors
           3448898336 for 512 sectors
           3458897888 for 512 sectors
           3458898400 for 512 sectors
           3519403992 for 512 sectors
           3519404504 for 512 sectors
           3617207456 for 512 sectors
           3617207968 for 512 sectors

Is it normal for badblocks to propagate to newly added drives on the 
array?

I am doing a stress test on the array now (28 nodes of a cluster all 
creating large files on the array) to see if it falls over again.

On 2015-11-09 17:35, Shaohua Li wrote:
> On Mon, Nov 09, 2015 at 11:40:00AM +0000, matt@digitallyhosted.com 
> wrote:
>> Hello,
>> 
>> I am experiencing issues with RAID6 on all kernel versions I have 
>> tried
>> (3.18.12, 4.0.9, 4.1.12).
>> 
>> On 3.18.12, I am getting the following logged to dmesg:
>> 
>> 896.874943] EXT4-fs warning (device md4): ext4_end_bio:317: I/O error 
>> -5
>> writing to inode 361858058 (offset 16777216 size 1052672 starting 
>> block
>> 5172953088)
>> [  896.874945] Buffer I/O error on device md4, logical block 
>> 5172953088
>> [  896.874947] Buffer I/O error on device md4, logical block 
>> 5172953089
>> [  896.874948] Buffer I/O error on device md4, logical block 
>> 5172953090
>> [  896.874949] Buffer I/O error on device md4, logical block 
>> 5172953091
>> [  896.874950] Buffer I/O error on device md4, logical block 
>> 5172953092
>> [  896.874950] Buffer I/O error on device md4, logical block 
>> 5172953093
>> [  896.874951] Buffer I/O error on device md4, logical block 
>> 5172953094
>> [  896.874952] Buffer I/O error on device md4, logical block 
>> 5172953095
>> [  896.874953] Buffer I/O error on device md4, logical block 
>> 5172953096
>> [  896.874953] Buffer I/O error on device md4, logical block 
>> 5172953097
>> [  897.034829] EXT4-fs warning (device md4): ext4_end_bio:317: I/O 
>> error -5
>> writing to inode 361858073 (offset 8388608 size 1052672 starting block
>> 5172955136)
>> [  897.122306] EXT4-fs warning (device md4): ext4_end_bio:317: I/O 
>> error -5
>> writing to inode 361858073 (offset 8388608 size 2101248 starting block
>> 5172955264)
>> [  897.130547] EXT4-fs warning (device md4): ext4_end_bio:317: I/O 
>> error -5
>> writing to inode 361858073 (offset 8388608 size 2101248 starting block
>> 5172955392)
>> [  897.355966] EXT4-fs warning (device md4): ext4_end_bio:317: I/O 
>> error -5
>> writing to inode 361858073 (offset 8388608 size 2625536 starting block
>> 5172955520)
>> [  897.452464] EXT4-fs warning (device md4): ext4_end_bio:317: I/O 
>> error -5
>> writing to inode 361858058 (offset 16777216 size 1576960 starting 
>> block
>> 5172953216)
>> [  897.593480] EXT4-fs warning (device md4): ext4_end_bio:317: I/O 
>> error -5
>> writing to inode 361858073 (offset 8388608 size 3149824 starting block
>> 5172955648)
>> [  897.877728] EXT4-fs warning (device md4): ext4_end_bio:317: I/O 
>> error -5
>> writing to inode 361858073 (offset 8388608 size 3674112 starting block
>> 5172955776)
>> [  898.156331] EXT4-fs warning (device md4): ext4_end_bio:317: I/O 
>> error -5
>> writing to inode 361858073 (offset 8388608 size 4198400 starting block
>> 5172955904)
>> [  898.176687] EXT4-fs warning (device md4): ext4_end_bio:317: I/O 
>> error -5
>> writing to inode 361858058 (offset 16777216 size 2101248 starting 
>> block
>> 5172953344)
>> 
>> When this happens, I end up with a file on the array which is 
>> partially
>> corrupt.  For example, if i copied a jpeg file, parts of the image 
>> would be
>> garbage.
>> 
>> I initially thought that this could be a kernel issue, so I tried two
>> further kernel versions (4.0.9 & 4.1.12) and on both, I don't get the 
>> above
>> messages anymore, instead I get a kernel oops and any process 
>> accessing the
>> array will get stuck in state D.  Here is a typical kernel oops 
>> message:
>> 
>> [  158.138253] BUG: unable to handle kernel NULL pointer dereference 
>> at
>> 0000000000000120
>> [  158.138391] IP: [<ffffffffa024cc1f>] handle_stripe+0xdc0/0x1e1f 
>> [raid456]
>> [  158.138482] PGD 24ff59067 PUD 24fe43067 PMD 0
>> [  158.138646] Oops: 0000 [#1] SMP
>> [  158.138758] Modules linked in: ipv6 binfmt_misc joydev
>> x86_pkg_temp_thermal coretemp kvm_intel kvm microcode pcspkr video 
>> i2c_i801
>> thermal acpi_cpufreq fan battery rtc_cmos backlight processor 
>> thermal_sys
>> xhci_pci button xts gf128mul aes_x86_64 cbc sha256_generic
>> scsi_transport_iscsi multipath linear raid10 raid456 async_raid6_recov
>> async_memcpy async_pq async_xor xor async_tx raid6_pq raid1 raid0
>> dm_snapshot dm_bufio dm_crypt dm_mirror dm_region_hash dm_log dm_mod
>> hid_sunplus hid_sony led_class hid_samsung hid_pl hid_petalynx 
>> hid_monterey
>> hid_microsoft hid_logitech hid_gyration hid_ezkey hid_cypress 
>> hid_chicony
>> hid_cherry hid_belkin hid_apple hid_a4tech sl811_hcd usbhid xhci_hcd
>> ohci_hcd uhci_hcd usb_storage ehci_pci ehci_hcd usbcore usb_common
>> megaraid_sas megaraid_mbox megaraid_mm megaraid sx8
>> [  158.141809]  DAC960 cciss mptsas mptfc scsi_transport_fc mptspi
>> scsi_transport_spi mptscsih mptbase sg
>> [  158.142226] CPU: 0 PID: 2017 Comm: md4_raid6 Not tainted 
>> 4.1.12-gentoo #1
>> [  158.142272] Hardware name: Supermicro X10SAT/X10SAT, BIOS 2.0 
>> 04/21/2014
>> [  158.142323] task: ffff880254267050 ti: ffff880095afc000 task.ti:
>> ffff880095afc000
>> [  158.142376] RIP: 0010:[<ffffffffa024cc1f>]  [<ffffffffa024cc1f>]
>> handle_stripe+0xdc0/0x1e1f [raid456]
>> [  158.142493] RSP: 0018:ffff880095affc18 EFLAGS: 00010202
>> [  158.142554] RAX: 000000000000000d RBX: ffff880095cfac00 RCX:
>> 0000000000000002
>> [  158.142617] RDX: 000000000000000d RSI: 0000000000000000 RDI:
>> 0000000000001040
>> [  158.142682] RBP: ffff880095affcf8 R08: 0000000000000003 R09:
>> 00000000cd920408
>> [  158.142745] R10: 000000000000000d R11: 0000000000000007 R12:
>> 000000000000000d
>> [  158.142809] R13: 0000000000000000 R14: 000000000000000c R15:
>> ffff8802161f2588
>> [  158.142873] FS:  0000000000000000(0000) GS:ffff88025ea00000(0000)
>> knlGS:0000000000000000
>> [  158.142938] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [  158.143000] CR2: 0000000000000120 CR3: 0000000253ef4000 CR4:
>> 00000000001406f0
>> [  158.143062] Stack:
>> [  158.143117]  0000000000000000 ffff880254267050 00000000000147c0
>> 0000000000000000
>> [  158.143328]  ffff8802161f25d0 0000000effffffff ffff8802161f3670
>> ffff8802161f2ef0
>> [  158.143537]  0000000000000000 0000000000000000 0000000000000000
>> 0000000c00000000
>> [  158.143747] Call Trace:
>> [  158.143805]  [<ffffffffa024dea3>]
>> handle_active_stripes.isra.37+0x225/0x2aa [raid456]
>> [  158.143873]  [<ffffffffa024e31d>] raid5d+0x363/0x40d [raid456]
>> [  158.143937]  [<ffffffff814315bc>] ? schedule+0x6f/0x7e
>> [  158.143998]  [<ffffffff81372ae7>] md_thread+0x125/0x13b
>> [  158.144060]  [<ffffffff81061b00>] ? wait_woken+0x71/0x71
>> [  158.144122]  [<ffffffff813729c2>] ? md_start_sync+0xda/0xda
>> [  158.144185]  [<ffffffff81050609>] kthread+0xcd/0xd5
>> [  158.144244]  [<ffffffff8105053c>] ? 
>> kthread_create_on_node+0x16d/0x16d
>> [  158.144309]  [<ffffffff81434f92>] ret_from_fork+0x42/0x70
>> [  158.144370]  [<ffffffff8105053c>] ? 
>> kthread_create_on_node+0x16d/0x16d
>> [  158.144432] Code: 8c 0f d0 01 00 00 48 8b 49 10 80 e1 10 74 0d 49 
>> 8b 4f
>> 48 80 e1 40 0f 84 c2 0f 00 00 31 c9 41 39 c8 7e 31 48 8b b4 cd 50 ff 
>> ff ff
>> <48> 83 be 20 01 00 00 00 74 1a 48 8b be 38 01 00 00 40 80 e7 01
>> [  158.147700] RIP [<ffffffffa024cc1f>] handle_stripe+0xdc0/0x1e1f 
>> [raid456]
>> [  158.147801]  RSP <ffff880095affc18>
>> [  158.147859] CR2: 0000000000000120
>> [  158.147916] ---[ end trace 536b72bd7c91f068 ]---
>> 
>> In both cases, discs are never flagged as faulty and the array never 
>> goes
>> into a degraded state.
>> 
>> I have tried posting this in various forums with no solution so far.  
>> A post
>> with further information can be found here:
>> https://forums.gentoo.org/viewtopic-t-1032304.html - In that topic I 
>> have
>> supplied output from various commands that people have asked me to 
>> execute.
>> Rather than pasting all the output from these commands here have 
>> linked to
>> the thread instead.
>> 
>> Any Idea's what could be going on? Any help would be greatly 
>> appreciated.
> 
> Could you please try a upstream kernel? there are some fixes in error 
> handling
> side recently, might be related.
> ebda780bce8d58ec0ab
> 36707bb2e7c6730d79

^ permalink raw reply

* Re: [PATCH 0/2] Introduce the request handling for dm-crypt
From: Christoph Hellwig @ 2015-11-11  9:48 UTC (permalink / raw)
  To: Baolin Wang
  Cc: axboe, agk, snitzer, dm-devel, neilb, jack, tj, jmoyer,
	keith.busch, bart.vanassche, linux-raid, broonie, arnd, dineshg,
	linux-kernel
In-Reply-To: <cover.1447233227.git.baolin.wang@linaro.org>

On Wed, Nov 11, 2015 at 05:31:43PM +0800, Baolin Wang wrote:
> Now the dm-crypt code only implemented the 'based-bio' method to encrypt/
> decrypt block data, which can only hanle one bio at one time. As we know,
> one bio must use the sequential physical address and it also has a limitation
> of length. Thus it may limit the big block encyrtion/decryption when some
> hardware support the big block data encryption.
> 
> This patch series introduc the 'based-request' method to handle the data
> encryption/decryption. One request can contain multiple bios, so it can
> handle big block data to improve the efficiency.

NAK for more request based stacking or DM drivers.  They are a major
pain to deal with, and adding more with different requirements then
dm-multipath is not helping in actually making that one work properly.

^ permalink raw reply

* [PATCH 2/2] md: dm-crypt: Introduce the request handling for dm-crypt
From: Baolin Wang @ 2015-11-11  9:31 UTC (permalink / raw)
  To: axboe, agk, snitzer, dm-devel, neilb
  Cc: jack, tj, jmoyer, keith.busch, bart.vanassche, linux-raid,
	broonie, arnd, dineshg, linux-kernel, baolin.wang
In-Reply-To: <cover.1447233227.git.baolin.wang@linaro.org>

Some hardware can support big block data encrytion, the original dm-crypt
only implemented the 'based-bio' things that will limit the efficiency
(only handle one bio at one time) for the big block data encryption.

This patch introduces the 'based-request' method to handle the big block,
which it can contain more than one bio at one time for dm-drypt. Now we use
a config macro to enable the 'based-request' method and to ensure the original
code can be run successfully.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 drivers/md/Kconfig    |    6 +
 drivers/md/dm-crypt.c |  831 ++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 835 insertions(+), 2 deletions(-)

diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
index d5415ee..aea1db0 100644
--- a/drivers/md/Kconfig
+++ b/drivers/md/Kconfig
@@ -266,6 +266,12 @@ config DM_CRYPT
 
 	  If unsure, say N.
 
+config DM_REQ_CRYPT
+	bool "Crypt target support with request"
+	depends on BLK_DEV_DM
+	select CRYPTO
+	select CRYPTO_CBC
+
 config DM_SNAPSHOT
        tristate "Snapshot target"
        depends on BLK_DEV_DM
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index d60c88d..e21a1ed15 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -28,10 +28,13 @@
 #include <crypto/hash.h>
 #include <crypto/md5.h>
 #include <crypto/algapi.h>
+#include <linux/buffer_head.h>
 
 #include <linux/device-mapper.h>
 
 #define DM_MSG_PREFIX "crypt"
+#define DM_MAX_SG_LIST	(1024)
+#define BIO_INLINE_VECS	(4)
 
 /*
  * context holding the current state of a multi-part conversion
@@ -64,10 +67,27 @@ struct dm_crypt_io {
 	struct rb_node rb_node;
 } CRYPTO_MINALIGN_ATTR;
 
+struct dm_req_crypt_io {
+	struct crypt_config *cc;
+	struct work_struct work;
+	struct request *cloned_request;
+	struct convert_context ctx;
+
+	int error;
+	atomic_t pending;
+	sector_t sector;
+	struct rb_node rb_node;
+
+	bool should_encrypt;
+	bool should_decrypt;
+};
+
 struct dm_crypt_request {
 	struct convert_context *ctx;
 	struct scatterlist sg_in;
 	struct scatterlist sg_out;
+	struct sg_table req_sgt_in;
+	struct sg_table req_sgt_out;
 	sector_t iv_sector;
 };
 
@@ -127,6 +147,10 @@ struct crypt_config {
 	 */
 	mempool_t *req_pool;
 	mempool_t *page_pool;
+
+	struct kmem_cache *req_crypt_io_pool;
+	mempool_t *req_io_pool;
+
 	struct bio_set *bs;
 	struct mutex bio_alloc_lock;
 
@@ -184,6 +208,7 @@ struct crypt_config {
 static void clone_init(struct dm_crypt_io *, struct bio *);
 static void kcryptd_queue_crypt(struct dm_crypt_io *io);
 static u8 *iv_of_dmreq(struct crypt_config *cc, struct dm_crypt_request *dmreq);
+static int req_crypt_write_work(void *data);
 
 /*
  * Use this to access cipher attributes that are the same for each CPU.
@@ -1547,6 +1572,8 @@ static void crypt_dtr(struct dm_target *ti)
 		mempool_destroy(cc->page_pool);
 	if (cc->req_pool)
 		mempool_destroy(cc->req_pool);
+	if (cc->req_io_pool)
+		mempool_destroy(cc->req_io_pool);
 
 	if (cc->iv_gen_ops && cc->iv_gen_ops->dtr)
 		cc->iv_gen_ops->dtr(cc);
@@ -1556,6 +1583,7 @@ static void crypt_dtr(struct dm_target *ti)
 
 	kzfree(cc->cipher);
 	kzfree(cc->cipher_string);
+	kmem_cache_destroy(cc->req_crypt_io_pool);
 
 	/* Must zero key material before freeing */
 	kzfree(cc);
@@ -1796,7 +1824,19 @@ static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 		goto bad;
 	}
 
-	cc->bs = bioset_create(MIN_IOS, 0);
+	cc->req_crypt_io_pool = KMEM_CACHE(dm_req_crypt_io, 0);
+	if (!cc->req_crypt_io_pool) {
+		ti->error = "Cannot allocate req_crypt_io_pool";
+		goto bad;
+	}
+
+	cc->req_io_pool = mempool_create_slab_pool(MIN_IOS, cc->req_crypt_io_pool);
+	if (!cc->req_io_pool) {
+		ti->error = "Cannot allocate request io mempool";
+		goto bad;
+	}
+
+	cc->bs = bioset_create(BIO_MAX_PAGES, 0);
 	if (!cc->bs) {
 		ti->error = "Cannot allocate crypt bioset";
 		goto bad;
@@ -1880,7 +1920,12 @@ static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 	init_waitqueue_head(&cc->write_thread_wait);
 	cc->write_tree = RB_ROOT;
 
+#ifndef CONFIG_DM_REQ_CRYPT
 	cc->write_thread = kthread_create(dmcrypt_write, cc, "dmcrypt_write");
+#else
+	cc->write_thread = kthread_create(req_crypt_write_work,
+					  cc, "req_dmcrypt_write");
+#endif
 	if (IS_ERR(cc->write_thread)) {
 		ret = PTR_ERR(cc->write_thread);
 		cc->write_thread = NULL;
@@ -2045,14 +2090,796 @@ static int crypt_iterate_devices(struct dm_target *ti,
 	return fn(ti, cc->dev, cc->start, ti->len, data);
 }
 
+/*
+ * If bio->bi_dev is a partition, remap the location
+ */
+static inline void req_crypt_blk_partition_remap(struct bio *bio)
+{
+	struct block_device *bdev = bio->bi_bdev;
+
+	if (bio_sectors(bio) && bdev != bdev->bd_contains) {
+		struct hd_struct *p = bdev->bd_part;
+		/* Check for integer overflow, should never happen. */
+		if (p->start_sect > (UINT_MAX - bio->bi_iter.bi_sector))
+			return;
+
+		bio->bi_iter.bi_sector += p->start_sect;
+		bio->bi_bdev = bdev->bd_contains;
+	}
+}
+
+static void req_crypt_dispatch_io(struct dm_req_crypt_io *io)
+{
+	struct request *clone = io->cloned_request;
+	struct request *rq = dm_get_orig_rq(clone);
+
+	dm_dispatch_clone_request(clone, rq);
+}
+
+static void req_crypt_free_resource(struct dm_req_crypt_io *io)
+{
+	struct crypt_config *cc = io->cc;
+	struct ablkcipher_request *req = io->ctx.req;
+	struct dm_crypt_request *dmreq = dmreq_of_req(cc, req);
+
+	if (dmreq->req_sgt_out.orig_nents > 0)
+		sg_free_table(&dmreq->req_sgt_out);
+
+	if (dmreq->req_sgt_in.orig_nents > 0)
+		sg_free_table(&dmreq->req_sgt_in);
+
+	mempool_free(req, cc->req_pool);
+	mempool_free(io, cc->req_io_pool);
+}
+
+static void req_crypt_inc_pending(struct dm_req_crypt_io *io)
+{
+	atomic_inc(&io->pending);
+}
+
+static void req_crypt_dec_pending_encrypt(struct dm_req_crypt_io *io)
+{
+	struct request *clone = io->cloned_request;
+	int error = io->error;
+
+	atomic_dec(&io->pending);
+
+	if (error < 0) {
+		dm_kill_unmapped_request(clone, error);
+		req_crypt_free_resource(io);
+	}
+}
+
+static void req_crypt_dec_pending_decrypt(struct dm_req_crypt_io *io)
+{
+	struct request *clone = io->cloned_request;
+	int error = io->error;
+
+	atomic_dec(&io->pending);
+
+	dm_end_request(clone, error);
+	req_crypt_free_resource(io);
+}
+
+/*
+ * This callback is called by the worker queue to perform non-decrypt writes
+ * and use the dm function to complete the bios and requests.
+ */
+static void req_crypt_write_plain(struct dm_req_crypt_io *io)
+{
+	io->error = 0;
+	req_crypt_dispatch_io(io);
+}
+
+/*
+ * This callback is called by the worker queue to perform non-decrypt reads
+ * and use the dm function to complete the bios and requests.
+ */
+static void req_crypt_read_plain(struct dm_req_crypt_io *io)
+{
+	struct crypt_config *cc = io->cc;
+	struct request *clone = io->cloned_request;
+
+	dm_end_request(clone, 0);
+	mempool_free(io, cc->req_io_pool);
+}
+
+#define req_crypt_io_from_node(node) rb_entry((node), struct dm_req_crypt_io, rb_node)
+static int req_crypt_write_work(void *data)
+{
+	struct crypt_config *cc = data;
+	struct dm_req_crypt_io *io;
+
+	while (1) {
+		struct rb_root write_tree;
+		struct blk_plug plug;
+		DECLARE_WAITQUEUE(wait, current);
+
+		spin_lock_irq(&cc->write_thread_wait.lock);
+
+continue_locked:
+		if (!RB_EMPTY_ROOT(&cc->write_tree))
+			goto pop_from_list;
+
+		__set_current_state(TASK_INTERRUPTIBLE);
+		__add_wait_queue(&cc->write_thread_wait, &wait);
+
+		spin_unlock_irq(&cc->write_thread_wait.lock);
+
+		if (unlikely(kthread_should_stop())) {
+			set_task_state(current, TASK_RUNNING);
+			remove_wait_queue(&cc->write_thread_wait, &wait);
+			break;
+		}
+
+		schedule();
+
+		set_task_state(current, TASK_RUNNING);
+		spin_lock_irq(&cc->write_thread_wait.lock);
+		__remove_wait_queue(&cc->write_thread_wait, &wait);
+		goto continue_locked;
+
+pop_from_list:
+		write_tree = cc->write_tree;
+		cc->write_tree = RB_ROOT;
+		spin_unlock_irq(&cc->write_thread_wait.lock);
+
+		BUG_ON(rb_parent(write_tree.rb_node));
+
+		blk_start_plug(&plug);
+		do {
+			io = req_crypt_io_from_node(rb_first(&write_tree));
+			rb_erase(&io->rb_node, &write_tree);
+			req_crypt_dispatch_io(io);
+		} while (!RB_EMPTY_ROOT(&write_tree));
+		blk_finish_plug(&plug);
+	}
+
+	return 0;
+}
+
+static void req_crypt_write_io_submit(struct dm_req_crypt_io *io, int async)
+{
+	struct crypt_config *cc = io->cc;
+	unsigned long flags;
+	sector_t sector;
+	struct rb_node **rbp, *parent;
+
+	if (io->error < 0)
+		return;
+
+	if (likely(!async) && test_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags)) {
+		req_crypt_dispatch_io(io);
+		return;
+	}
+
+	spin_lock_irqsave(&cc->write_thread_wait.lock, flags);
+	rbp = &cc->write_tree.rb_node;
+	parent = NULL;
+	sector = io->sector;
+
+	while (*rbp) {
+		parent = *rbp;
+		if (sector < req_crypt_io_from_node(parent)->sector)
+			rbp = &(*rbp)->rb_left;
+		else
+			rbp = &(*rbp)->rb_right;
+	}
+
+	rb_link_node(&io->rb_node, parent, rbp);
+	rb_insert_color(&io->rb_node, &cc->write_tree);
+
+	wake_up_locked(&cc->write_thread_wait);
+	spin_unlock_irqrestore(&cc->write_thread_wait.lock, flags);
+}
+
+/*
+ * Cipher complete callback, this is triggered by the linux crypto api once
+ * the operation is done. This signals the waiting thread that the crypto
+ * operation is complete.
+ */
+static void req_crypt_cipher_complete(struct crypto_async_request *req, int err)
+{
+	struct dm_crypt_request *dmreq = req->data;
+	struct convert_context *ctx = dmreq->ctx;
+	struct dm_req_crypt_io *io =
+		container_of(ctx, struct dm_req_crypt_io, ctx);
+	struct crypt_config *cc = io->cc;
+
+	if (err == -EINPROGRESS)
+		return;
+
+	io->error = err;
+	atomic_dec(&io->ctx.cc_pending);
+	complete(&io->ctx.restart);
+
+	if (!err && cc->iv_gen_ops && cc->iv_gen_ops->post)
+		err = cc->iv_gen_ops->post(cc, iv_of_dmreq(cc, dmreq), dmreq);
+}
+
+static int req_crypt_alloc_req(struct crypt_config *cc,
+				struct convert_context *ctx)
+{
+	/* TODO: need to reconsider and modify here */
+	unsigned int key_index = ctx->cc_sector & (cc->tfms_count - 1);
+	struct dm_crypt_request *dmreq;
+
+	ctx->req = mempool_alloc(cc->req_pool, GFP_NOIO);
+	if (!ctx->req)
+		return -ENOMEM;
+
+	dmreq = dmreq_of_req(cc, ctx->req);
+	dmreq->req_sgt_in.orig_nents = 0;
+	dmreq->req_sgt_out.orig_nents = 0;
+
+	crypto_ablkcipher_clear_flags(cc->tfms[key_index], ~0);
+	ablkcipher_request_set_tfm(ctx->req, cc->tfms[key_index]);
+
+	/*
+	 * Use REQ_MAY_BACKLOG so a cipher driver internally backlogs
+	 * requests if driver request queue is full.
+	 */
+	ablkcipher_request_set_callback(ctx->req,
+	    CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
+	    req_crypt_cipher_complete, dmreq_of_req(cc, ctx->req));
+
+	return 0;
+}
+
+/*
+ * Free the pages that used to allacation for write operation, also it
+ * will free the bvec if there are.
+ */
+static void req_crypt_free_pages(struct crypt_config *cc, struct request *clone)
+{
+	struct req_iterator iter;
+	struct bio_vec bvec;
+	struct bio *bio_t;
+	int nr_iovecs = 0;
+
+	rq_for_each_segment(bvec, clone, iter) {
+		if (bvec.bv_offset == 0 && bvec.bv_page)
+			mempool_free(bvec.bv_page, cc->page_pool);
+		bvec.bv_page = NULL;
+	}
+
+	__rq_for_each_bio(bio_t, clone) {
+		nr_iovecs = bio_t->bi_max_vecs;
+		if (nr_iovecs > BIO_INLINE_VECS) {
+			BIO_BUG_ON(BIO_POOL_IDX(bio_t) >= BIOVEC_NR_POOLS);
+			bvec_free(cc->bs->bvec_pool, bio_t->bi_io_vec,
+				  BIO_POOL_IDX(bio_t));
+		}
+	}
+}
+
+/*
+ * Allocate the pages for write operation.
+ */
+static int req_crypt_alloc_pages(struct crypt_config *cc, struct request *clone)
+{
+	gfp_t gfp_mask = GFP_NOWAIT | __GFP_HIGHMEM;
+	struct page *page = NULL;
+	struct bio_vec *bvl = NULL;
+	struct bio_vec *bv = NULL;
+	struct bio *bio_t = NULL;
+	unsigned long idx = BIO_POOL_NONE;
+	struct bio_vec bvec;
+	struct bvec_iter biter;
+	int nr_iovecs = 0, i = 0, remaining_size = 0;
+
+	/*
+	 * When clone the request, it will not copy the bi_vcnt and
+	 * bi_max_vecs of one bio, so we should set it here.
+	 */
+	__rq_for_each_bio(bio_t, clone) {
+		nr_iovecs = 0;
+		bio_for_each_segment(bvec, bio_t, biter)
+			nr_iovecs++;
+		bio_t->bi_vcnt = bio_t->bi_max_vecs = nr_iovecs;
+	}
+
+	/*
+	 * When clone the original request, it will also clone the bios of
+	 * the original request. But it will not copy the pages which the
+	 * original bios are pointing to and the cloned bios just point
+	 * same page. So here we need to allocate some new pages for the
+	 * clone bios to encrypto system.
+	 */
+	__rq_for_each_bio(bio_t, clone) {
+		nr_iovecs = bio_t->bi_max_vecs;
+		if (nr_iovecs > BIO_INLINE_VECS)
+			bvl = bvec_alloc(GFP_NOIO, nr_iovecs,
+					 &idx, cc->bs->bvec_pool);
+		else if (nr_iovecs)
+			bvl = bio_t->bi_inline_vecs;
+
+		if (!bvl)
+			return -ENOMEM;
+
+		memcpy(bvl, bio_t->bi_io_vec,
+		       nr_iovecs * sizeof(struct bio_vec));
+		bio_t->bi_max_vecs = nr_iovecs;
+		bio_t->bi_io_vec = bvl;
+		if (idx < BIO_POOL_NONE) {
+			bio_t->bi_flags &= ~(BIO_POOL_NONE << BIO_POOL_OFFSET);
+			bio_t->bi_flags |= idx << BIO_POOL_OFFSET;
+		}
+	}
+
+	__rq_for_each_bio(bio_t, clone) {
+		bio_for_each_segment_all(bv, bio_t, i) {
+			if (bv->bv_len > remaining_size) {
+				page = NULL;
+				while (page == NULL) {
+					page = mempool_alloc(cc->page_pool,
+							     gfp_mask);
+					if (!page) {
+						DMERR("%s page alloc failed",
+						      __func__);
+						congestion_wait(BLK_RW_ASYNC,
+								HZ/100);
+					}
+				}
+
+				bv->bv_page = page;
+				bv->bv_offset = 0;
+				remaining_size = PAGE_SIZE - bv->bv_len;
+				if (remaining_size < 0)
+					BUG();
+			} else {
+				bv->bv_page = page;
+				bv->bv_offset = PAGE_SIZE - remaining_size;
+				remaining_size = remaining_size - bv->bv_len;
+			}
+		}
+	}
+
+	return 0;
+}
+
+/*
+ * Check how many sg entry numbers are needed when map one request
+ * with scatterlist in advance.
+ */
+static unsigned int req_crypt_clone_sg_entry(struct request *clone)
+{
+	struct request_queue *q = clone->q;
+	struct bio_vec bvec, bvprv = { NULL };
+	struct bio *bio_t = NULL;
+	struct bvec_iter biter;
+	unsigned int nbytes, sg_length, sg_cnt = 0;
+
+	__rq_for_each_bio(bio_t, clone) {
+		sg_length = 0;
+		bio_for_each_segment(bvec, bio_t, biter) {
+			nbytes = bvec.bv_len;
+			if (sg_length + nbytes > queue_max_segment_size(q)) {
+				sg_length = 0;
+				sg_cnt++;
+				goto next;
+			}
+
+			if (!BIOVEC_PHYS_MERGEABLE(&bvprv, &bvec)) {
+				sg_length = 0;
+				sg_cnt++;
+				goto next;
+			}
+
+			if (!BIOVEC_SEG_BOUNDARY(q, &bvprv, &bvec)) {
+				sg_length = 0;
+				sg_cnt++;
+				goto next;
+			}
+
+			sg_length += nbytes;
+next:
+			memcpy(&bvprv, &bvec, sizeof(struct bio_vec));
+		}
+	}
+
+	return sg_cnt;
+}
+
+static int req_crypt_convert_block(struct crypt_config *cc,
+				   struct request *clone,
+				   struct convert_context *ctx)
+{
+	struct ablkcipher_request *req = ctx->req;
+	struct dm_crypt_request *dmreq = dmreq_of_req(cc, req);
+	u8 *iv = iv_of_dmreq(cc, dmreq);
+	struct scatterlist *req_sg_in = NULL;
+	struct scatterlist *req_sg_out = NULL;
+	unsigned int total_sg_len_req_in = 0;
+	unsigned int total_sg_len_req_out = 0;
+	unsigned int total_bytes_in_req = 0;
+	unsigned int sg_in_max = 0, sg_out_max = 0;
+	int ret;
+
+	dmreq->iv_sector = ctx->cc_sector;
+	dmreq->ctx = ctx;
+	atomic_set(&ctx->cc_pending, 1);
+
+	/*
+	 * Need to calculate how many sg entry need to be used
+	 * for this clone.
+	 */
+	sg_in_max = req_crypt_clone_sg_entry(clone) + 1;
+	if (sg_in_max > DM_MAX_SG_LIST || sg_in_max <= 0) {
+		DMERR("%s sg entry too large or none %d\n",
+		      __func__, sg_in_max);
+		return -EINVAL;
+	} else if (sg_in_max == 2) {
+		req_sg_in = &dmreq->sg_in;
+	}
+
+	if (!req_sg_in) {
+		ret = sg_alloc_table(&dmreq->req_sgt_in,
+				     sg_in_max, GFP_KERNEL);
+		if (ret) {
+			DMERR("%s sg in allocation failed\n", __func__);
+			return -ENOMEM;
+		}
+
+		req_sg_in = dmreq->req_sgt_in.sgl;
+	}
+
+	total_sg_len_req_in = blk_rq_map_sg(clone->q, clone, req_sg_in);
+	if ((total_sg_len_req_in <= 0)
+	    || (total_sg_len_req_in > sg_in_max)) {
+		DMERR("%s in sg map error %d\n", __func__, total_sg_len_req_in);
+		return -EINVAL;
+	}
+
+	total_bytes_in_req = clone->__data_len;
+
+	if (rq_data_dir(clone) == READ)
+		goto set_crypt;
+
+	ret = req_crypt_alloc_pages(cc, clone);
+	if (ret < 0) {
+		DMERR("%s alloc request pages failed\n", __func__);
+		return -ENOMEM;
+	}
+
+	sg_out_max = req_crypt_clone_sg_entry(clone) + 1;
+	if (sg_out_max > DM_MAX_SG_LIST || sg_out_max <= 0) {
+		DMERR("%s sg entry too large or none %d\n",
+		      __func__, sg_out_max);
+		return -EINVAL;
+	} else if (sg_out_max == 2) {
+		req_sg_out = &dmreq->sg_out;
+	}
+
+	if (!req_sg_out) {
+		ret = sg_alloc_table(&dmreq->req_sgt_out,
+				     sg_out_max, GFP_KERNEL);
+		if (ret) {
+			DMERR("%s sg out allocation failed\n", __func__);
+			return -ENOMEM;
+		}
+
+		req_sg_out = dmreq->req_sgt_out.sgl;
+	}
+
+	total_sg_len_req_out = blk_rq_map_sg(clone->q, clone, req_sg_out);
+	if ((total_sg_len_req_out <= 0) ||
+	    (total_sg_len_req_out > sg_out_max)) {
+		DMERR("%s out sg map error %d\n",
+		      __func__, total_sg_len_req_out);
+		return -EINVAL;
+	}
+
+set_crypt:
+	if (cc->iv_gen_ops) {
+		ret = cc->iv_gen_ops->generator(cc, iv, dmreq);
+		if (ret < 0) {
+			DMERR("%s generator iv error %d\n", __func__, ret);
+			return ret;
+		}
+	}
+
+	atomic_inc(&ctx->cc_pending);
+
+	if (rq_data_dir(clone) == WRITE) {
+		ablkcipher_request_set_crypt(req, req_sg_in,
+			req_sg_out, total_bytes_in_req, iv);
+
+		ret = crypto_ablkcipher_encrypt(req);
+	} else {
+		ablkcipher_request_set_crypt(req, req_sg_in,
+			req_sg_in, total_bytes_in_req, iv);
+
+		ret = crypto_ablkcipher_decrypt(req);
+	}
+
+	if (!ret && cc->iv_gen_ops && cc->iv_gen_ops->post)
+		ret = cc->iv_gen_ops->post(cc, iv, dmreq);
+
+	return ret;
+}
+
+static void req_crypt_write_convert(struct dm_req_crypt_io *io)
+{
+	struct request *clone = io->cloned_request;
+	struct bio *bio_src = NULL;
+	struct crypt_config *cc = io->cc;
+	int crypt_finished;
+	int ret = 0, err = 0;
+
+	req_crypt_inc_pending(io);
+
+	crypt_convert_init(cc, &io->ctx, NULL, NULL, io->sector);
+	req_crypt_alloc_req(cc, &io->ctx);
+
+	ret = req_crypt_convert_block(cc, clone, &io->ctx);
+	switch (ret) {
+	case 0:
+		atomic_dec(&io->ctx.cc_pending);
+		break;
+	case -EBUSY:
+		/*
+		 * Lets make this synchronous request by waiting on
+		 * in progress as well
+		 */
+	case -EINPROGRESS:
+		wait_for_completion_io(&io->ctx.restart);
+		if (io->error) {
+			err = -EIO;
+			goto crypt_error;
+		}
+		break;
+	default:
+		err = -EIO;
+		atomic_dec(&io->ctx.cc_pending);
+		break;
+	}
+
+	__rq_for_each_bio(bio_src, clone)
+		blk_queue_bounce(clone->q, &bio_src);
+
+crypt_error:
+	if (err == -EIO)
+		req_crypt_free_pages(cc, clone);
+
+	if (io)
+		io->error = err;
+
+	/* Encryption was already finished, submit io now */
+	crypt_finished = atomic_dec_and_test(&io->ctx.cc_pending);
+	if (crypt_finished)
+		req_crypt_write_io_submit(io, 0);
+	else
+		io->error = -EIO;
+
+	req_crypt_dec_pending_encrypt(io);
+}
+
+static void req_crypt_read_convert(struct dm_req_crypt_io *io)
+{
+	struct crypt_config *cc = io->cc;
+	struct request *clone = io->cloned_request;
+	int ret = 0, err = 0;
+
+	req_crypt_inc_pending(io);
+
+	/* io->sector need to be initilized */
+	crypt_convert_init(cc, &io->ctx, NULL, NULL, io->sector);
+	req_crypt_alloc_req(cc, &io->ctx);
+
+	ret = req_crypt_convert_block(cc, clone, &io->ctx);
+	switch (ret) {
+	case 0:
+		atomic_dec(&io->ctx.cc_pending);
+		break;
+	case -EBUSY:
+		/*
+		 * Lets make this synchronous request by waiting on
+		 * in progress as well
+		 */
+	case -EINPROGRESS:
+		wait_for_completion_io(&io->ctx.restart);
+		if (io->error)
+			err = -EIO;
+		break;
+	default:
+		err = -EIO;
+		atomic_dec(&io->ctx.cc_pending);
+		break;
+	}
+
+	if (io)
+		io->error = err;
+
+	if (!atomic_dec_and_test(&io->ctx.cc_pending))
+		DMWARN("%s decryption was not finished\n", __func__);
+
+	req_crypt_dec_pending_decrypt(io);
+}
+
+/* Queue callback function that will get triggered */
+static void req_crypt_work(struct work_struct *work)
+{
+	struct dm_req_crypt_io *io =
+			container_of(work, struct dm_req_crypt_io, work);
+
+	if (rq_data_dir(io->cloned_request) == WRITE) {
+		if (io->should_encrypt)
+			req_crypt_write_convert(io);
+		else
+			req_crypt_write_plain(io);
+	} else if (rq_data_dir(io->cloned_request) == READ) {
+		if (io->should_decrypt)
+			req_crypt_read_convert(io);
+		else
+			req_crypt_read_plain(io);
+	} else {
+		DMERR("%s received non-write request for clone 0x%p\n",
+		      __func__, io->cloned_request);
+	}
+}
+
+static void req_crypt_queue(struct dm_req_crypt_io *io)
+{
+	struct crypt_config *cc = io->cc;
+
+	INIT_WORK(&io->work, req_crypt_work);
+	queue_work(cc->crypt_queue, &io->work);
+}
+
+static bool req_crypt_should_encrypt(struct dm_req_crypt_io *req)
+{
+	if (!req || !req->cloned_request || !req->cloned_request->bio)
+		return false;
+
+	/* Maybe there are some others to be considerated */
+	return true;
+}
+
+static bool req_crypt_should_deccrypt(struct dm_req_crypt_io *req)
+{
+	if (!req || !req->cloned_request || !req->cloned_request->bio)
+		return false;
+
+	/* Maybe there are some others to be considerated */
+	return true;
+}
+
+static void crypt_req_io_init(struct dm_req_crypt_io *io,
+			      struct crypt_config *cc,
+			      struct request *clone,
+			      sector_t sector)
+{
+	io->cc = cc;
+	io->sector = sector;
+	io->cloned_request = clone;
+	io->error = 0;
+	io->ctx.req = NULL;
+	atomic_set(&io->pending, 0);
+
+	if (rq_data_dir(clone) == WRITE)
+		io->should_encrypt = req_crypt_should_encrypt(io);
+	else if (rq_data_dir(clone) == READ)
+		io->should_decrypt = req_crypt_should_deccrypt(io);
+	else
+		io->should_decrypt = 0;
+}
+
+/*
+ * This function is called with interrupts disabled
+ * The function remaps the clone for the underlying device.
+ * If it is a write request, it calls into the worker queue to
+ * encrypt the data
+ * and submit the request directly using the elevator
+ * For a read request no pre-processing is required the request
+ * is returned to dm once mapping is done
+ */
+static int req_crypt_map(struct dm_target *ti, struct request *clone,
+			 union map_info *map_context)
+{
+	struct crypt_config *cc = ti->private;
+	int copy_bio_sector_to_req = 0;
+	struct dm_req_crypt_io *req_io;
+	struct bio *bio_src;
+
+	if ((rq_data_dir(clone) != READ) && (rq_data_dir(clone) != WRITE)) {
+		DMERR("%s unknown request.\n", __func__);
+		return -EINVAL;
+	}
+
+	req_io = mempool_alloc(cc->req_io_pool, GFP_NOWAIT);
+	if (!req_io) {
+		DMERR("%s req io allocation failed.\n", __func__);
+		return -ENOMEM;
+	}
+
+	map_context->ptr = req_io;
+
+	/* Get the queue of the underlying original device */
+	clone->q = bdev_get_queue(cc->dev->bdev);
+	clone->rq_disk = cc->dev->bdev->bd_disk;
+
+	__rq_for_each_bio(bio_src, clone) {
+		bio_src->bi_bdev = cc->dev->bdev;
+		/*
+		 * If request is REQ_FLUSH or REQ_DISCARD, just bypass crypt
+		 * queues. It will free the bios of the request in block layer
+		 * when completing the bypass if the request is REQ_FLUSH or
+		 * REQ_DISCARD.
+		 */
+		if (clone->cmd_flags & REQ_DISCARD
+		    || clone->cmd_flags & REQ_FLUSH)
+			continue;
+
+		bio_set_flag(bio_src, BIO_ENDIO_FREE);
+
+		/*
+		 * If this device has partitions, remap block n
+		 * of partition p to block n+start(p) of the disk.
+		 */
+		req_crypt_blk_partition_remap(bio_src);
+		if (copy_bio_sector_to_req == 0) {
+			clone->__sector = bio_src->bi_iter.bi_sector;
+			copy_bio_sector_to_req++;
+		}
+		blk_queue_bounce(clone->q, &bio_src);
+	}
+
+	crypt_req_io_init(req_io, cc, clone,
+			  dm_target_offset(ti, clone->__sector));
+
+	if (rq_data_dir(clone) == READ) {
+		return DM_MAPIO_REMAPPED;
+	} else if (rq_data_dir(clone) == WRITE) {
+		req_crypt_queue(req_io);
+		return DM_MAPIO_SUBMITTED;
+	}
+
+	return -EINVAL;
+}
+
+/*
+ * The endio function is called from ksoftirqd context (atomic).
+ * For write operations the new pages created form the mempool
+ * is freed and returned.  * For read operations, decryption is
+ * required, since this is called in a atomic  * context, the
+ * request is sent to a worker queue to complete decryption and
+ * free the request once done.
+ */
+static int req_crypt_endio(struct dm_target *ti, struct request *clone,
+			   int error, union map_info *map_context)
+{
+	struct dm_req_crypt_io *req_io = map_context->ptr;
+	struct crypt_config *cc = ti->private;
+	int ret = 0;
+
+	/* If it is a write request, do nothing just return. */
+	if (rq_data_dir(clone) == WRITE) {
+		if (req_io->should_encrypt)
+			req_crypt_free_pages(cc, clone);
+		req_crypt_free_resource(req_io);
+	} else if (rq_data_dir(clone) == READ) {
+		req_io->error = error;
+		req_crypt_queue(req_io);
+		ret = DM_ENDIO_INCOMPLETE;
+	}
+
+	return ret;
+}
+
 static struct target_type crypt_target = {
 	.name   = "crypt",
 	.version = {1, 14, 0},
 	.module = THIS_MODULE,
 	.ctr    = crypt_ctr,
 	.dtr    = crypt_dtr,
-	.map    = crypt_map,
 	.status = crypt_status,
+#ifndef CONFIG_DM_REQ_CRYPT
+	.map    = crypt_map,
+#else
+	.map_rq = req_crypt_map,
+	.rq_end_io = req_crypt_endio,
+#endif
 	.postsuspend = crypt_postsuspend,
 	.preresume = crypt_preresume,
 	.resume = crypt_resume,
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH 1/2] block: Introduce BIO_ENDIO_FREE for bio flags
From: Baolin Wang @ 2015-11-11  9:31 UTC (permalink / raw)
  To: axboe, agk, snitzer, dm-devel, neilb
  Cc: jack, tj, jmoyer, keith.busch, bart.vanassche, linux-raid,
	broonie, arnd, dineshg, linux-kernel, baolin.wang
In-Reply-To: <cover.1447233227.git.baolin.wang@linaro.org>

When we use dm-crypt to decrypt block data, it will decrypt the block data
in endio() when one IO is completed. In this situation we don't want the
cloned bios is freed before calling the endio().

Thus introduce 'BIO_ENDIO_FREE' flag to support the request handling for dm-crypt,
this flag will ensure that blk layer does not complete the cloned bios before
completing the request. When the crypt endio is called, post-processsing is
done and then the dm layer will complete the bios (clones) and free them.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 block/blk-core.c              |    6 +++++-
 drivers/md/dm.c               |   13 ++++++++++---
 include/linux/blk_types.h     |    6 ++++++
 include/linux/device-mapper.h |    5 +++++
 4 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 60912e9..6838936 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1411,7 +1411,7 @@ void __blk_put_request(struct request_queue *q, struct request *req)
 	elv_completed_request(q, req);
 
 	/* this is a bio leak */
-	WARN_ON(req->bio != NULL);
+	WARN_ON(req->bio != NULL && !bio_flagged(req->bio, BIO_ENDIO_FREE));
 
 	/*
 	 * Request may not have originated from ll_rw_blk. if not,
@@ -2521,6 +2521,10 @@ bool blk_update_request(struct request *req, int error, unsigned int nr_bytes)
 	blk_account_io_completion(req, nr_bytes);
 
 	total_bytes = 0;
+
+	if (bio_flagged(req->bio, BIO_ENDIO_FREE))
+		return false;
+
 	while (req->bio) {
 		struct bio *bio = req->bio;
 		unsigned bio_bytes = min(bio->bi_iter.bi_size, nr_bytes);
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 6264781..2c18a34 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1047,6 +1047,13 @@ static struct dm_rq_target_io *tio_from_request(struct request *rq)
 	return (rq->q->mq_ops ? blk_mq_rq_to_pdu(rq) : rq->special);
 }
 
+struct request *dm_get_orig_rq(struct request *clone)
+{
+	struct dm_rq_target_io *tio = clone->end_io_data;
+
+	return tio->orig;
+}
+
 static void rq_end_stats(struct mapped_device *md, struct request *orig)
 {
 	if (unlikely(dm_stats_used(&md->stats))) {
@@ -1118,7 +1125,7 @@ static void free_rq_clone(struct request *clone)
  * Must be called without clone's queue lock held,
  * see end_clone_request() for more details.
  */
-static void dm_end_request(struct request *clone, int error)
+void dm_end_request(struct request *clone, int error)
 {
 	int rw = rq_data_dir(clone);
 	struct dm_rq_target_io *tio = clone->end_io_data;
@@ -1311,7 +1318,7 @@ static void dm_complete_request(struct request *rq, int error)
  * Target's rq_end_io() function isn't called.
  * This may be used when the target's map_rq() or clone_and_map_rq() functions fail.
  */
-static void dm_kill_unmapped_request(struct request *rq, int error)
+void dm_kill_unmapped_request(struct request *rq, int error)
 {
 	rq->cmd_flags |= REQ_FAILED;
 	dm_complete_request(rq, error);
@@ -1758,7 +1765,7 @@ int dm_request_based(struct mapped_device *md)
 	return blk_queue_stackable(md->queue);
 }
 
-static void dm_dispatch_clone_request(struct request *clone, struct request *rq)
+void dm_dispatch_clone_request(struct request *clone, struct request *rq)
 {
 	int r;
 
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index e813013..30aee54 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -122,6 +122,12 @@ struct bio {
 #define BIO_REFFED	8	/* bio has elevated ->bi_cnt */
 
 /*
+ * Added for Req based dm which need to perform post processing. This flag
+ * ensures blk_update_request does not free the bios or request, this is done
+ * at the dm level.
+ */
+#define BIO_ENDIO_FREE 12
+/*
  * Flags starting here get preserved by bio_reset() - this includes
  * BIO_POOL_IDX()
  */
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index 76d23fa..f636c50 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -407,6 +407,11 @@ union map_info *dm_get_rq_mapinfo(struct request *rq);
 
 struct queue_limits *dm_get_queue_limits(struct mapped_device *md);
 
+void dm_end_request(struct request *clone, int error);
+void dm_kill_unmapped_request(struct request *rq, int error);
+void dm_dispatch_clone_request(struct request *clone, struct request *rq);
+struct request *dm_get_orig_rq(struct request *clone);
+
 /*
  * Geometry functions.
  */
-- 
1.7.9.5

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox