[git pull] device mapper changes for 4.21

[git pull] device mapper changes for 4.21

[Mike Snitzer]

Hi Linus,

The DM tree is based on the block tree for this cycle because a fair
amount of my time was spent working on the percpu inflight IO counters
changes and other block fixes (which impact DM) that are in Jens' block
pull.

The following changes since commit cd19181bf9ad4b7f40f2a4e0355d052109c76529:

  blk-mq: enable IO poll if .nr_queues of type poll > 0 (2018-12-17 21:35:07 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git tags/for-4.21/dm-changes

for you to fetch changes up to c6d6e9b0f6b4201c77f2cea3964dd122697e3543:

  dm: do not allow readahead to limit IO size (2018-12-18 14:23:41 -0500)

Please pull, thanks.
Mike

----------------------------------------------------------------
- Eliminate a couple indirect calls from bio-based DM core.

- Fix DM to allow reads that exceed readahead limits by setting io_pages
  in the backing_dev_info.

- A couple code cleanups in request-based DM.

- Fix various DM targets to check for device sector overflow if
  CONFIG_LBDAF is not set.

- Use u64 instead of sector_t to store iv_offset in DM crypt; sector_t
  isn't large enough on 32bit when CONFIG_LBDAF is not set.

- Performance fixes to DM's kcopyd and the snapshot target focused on
  limiting memory use and workqueue stalls.

- Fix typos in the integrity and writecache targets.

- Log which algorithm is used for dm-crypt's encryption and
  dm-integrity's hashing.

- Fix false -EBUSY errors in DM raid target's handling of check/repair
  messages.

- Fix DM flakey target's corrupt_bio_byte feature to reliably corrupt
  the Nth byte in a bio's payload.

----------------------------------------------------------------
AliOS system security (1):
      dm crypt: use u64 instead of sector_t to store iv_offset

Colin Ian King (1):
      dm integrity: fix spelling mistake in workqueue name

Eric Biggers (2):
      dm crypt: log the encryption algorithm implementation
      dm verity: log the hash algorithm implementation

Heinz Mauelshagen (1):
      dm raid: fix false -EBUSY when handling check/repair message

Jaegeuk Kim (1):
      dm: do not allow readahead to limit IO size

Mike Snitzer (3):
      dm rq: remove unused arguments from rq_completed()
      dm: remove indirect calls from __send_changing_extent_only()
      dm rq: cleanup leftover code from recently removed q->mq_ops branching

Mikulas Patocka (1):
      dm: avoid indirect call in __dm_make_request

Milan Broz (1):
      dm: Check for device sector overflow if CONFIG_LBDAF is not set

Nikos Tsironis (2):
      dm snapshot: Fix excessive memory usage and workqueue stalls
      dm kcopyd: Fix bug causing workqueue stalls

Shenghui Wang (2):
      dm writecache: fix typo in error msg for creating writecache_flush_thread
      dm bufio: update comment in dm-bufio.c

Sweet Tea (1):
      dm flakey: Properly corrupt multi-page bios.

wuzhouhui (1):
      dm mpath: only flush workqueue when needed

 drivers/md/dm-bufio.c         | 12 +++++------
 drivers/md/dm-crypt.c         | 17 ++++++++++++----
 drivers/md/dm-delay.c         |  2 +-
 drivers/md/dm-flakey.c        | 35 +++++++++++++++++++++-----------
 drivers/md/dm-integrity.c     |  2 +-
 drivers/md/dm-kcopyd.c        | 19 +++++++++++++-----
 drivers/md/dm-linear.c        |  2 +-
 drivers/md/dm-mpath.c         |  6 ++++--
 drivers/md/dm-raid.c          |  3 +--
 drivers/md/dm-raid1.c         |  3 ++-
 drivers/md/dm-rq.c            | 18 ++++++-----------
 drivers/md/dm-snap.c          | 22 +++++++++++++++++++++
 drivers/md/dm-table.c         |  3 +++
 drivers/md/dm-unstripe.c      |  2 +-
 drivers/md/dm-verity-target.c |  9 +++++++++
 drivers/md/dm-writecache.c    |  2 +-
 drivers/md/dm.c               | 46 ++++++++++++-------------------------------
 17 files changed, 121 insertions(+), 82 deletions(-)

[git pull] device mapper changes for 4.21

[Mike Snitzer]

Hi Linus,

The DM tree is based on the block tree for this cycle because a fair
amount of my time was spent working on the percpu inflight IO counters
changes and other block fixes (which impact DM) that are in Jens' block
pull.

The following changes since commit cd19181bf9ad4b7f40f2a4e0355d052109c76529:

  blk-mq: enable IO poll if .nr_queues of type poll > 0 (2018-12-17 21:35:07 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git tags/for-4.21/dm-changes

for you to fetch changes up to c6d6e9b0f6b4201c77f2cea3964dd122697e3543:

  dm: do not allow readahead to limit IO size (2018-12-18 14:23:41 -0500)

Please pull, thanks.
Mike

----------------------------------------------------------------
- Eliminate a couple indirect calls from bio-based DM core.

- Fix DM to allow reads that exceed readahead limits by setting io_pages
  in the backing_dev_info.

- A couple code cleanups in request-based DM.

- Fix various DM targets to check for device sector overflow if
  CONFIG_LBDAF is not set.

- Use u64 instead of sector_t to store iv_offset in DM crypt; sector_t
  isn't large enough on 32bit when CONFIG_LBDAF is not set.

- Performance fixes to DM's kcopyd and the snapshot target focused on
  limiting memory use and workqueue stalls.

- Fix typos in the integrity and writecache targets.

- Log which algorithm is used for dm-crypt's encryption and
  dm-integrity's hashing.

- Fix false -EBUSY errors in DM raid target's handling of check/repair
  messages.

- Fix DM flakey target's corrupt_bio_byte feature to reliably corrupt
  the Nth byte in a bio's payload.

----------------------------------------------------------------
AliOS system security (1):
      dm crypt: use u64 instead of sector_t to store iv_offset

Colin Ian King (1):
      dm integrity: fix spelling mistake in workqueue name

Eric Biggers (2):
      dm crypt: log the encryption algorithm implementation
      dm verity: log the hash algorithm implementation

Heinz Mauelshagen (1):
      dm raid: fix false -EBUSY when handling check/repair message

Jaegeuk Kim (1):
      dm: do not allow readahead to limit IO size

Mike Snitzer (3):
      dm rq: remove unused arguments from rq_completed()
      dm: remove indirect calls from __send_changing_extent_only()
      dm rq: cleanup leftover code from recently removed q->mq_ops branching

Mikulas Patocka (1):
      dm: avoid indirect call in __dm_make_request

Milan Broz (1):
      dm: Check for device sector overflow if CONFIG_LBDAF is not set

Nikos Tsironis (2):
      dm snapshot: Fix excessive memory usage and workqueue stalls
      dm kcopyd: Fix bug causing workqueue stalls

Shenghui Wang (2):
      dm writecache: fix typo in error msg for creating writecache_flush_thread
      dm bufio: update comment in dm-bufio.c

Sweet Tea (1):
      dm flakey: Properly corrupt multi-page bios.

wuzhouhui (1):
      dm mpath: only flush workqueue when needed

 drivers/md/dm-bufio.c         | 12 +++++------
 drivers/md/dm-crypt.c         | 17 ++++++++++++----
 drivers/md/dm-delay.c         |  2 +-
 drivers/md/dm-flakey.c        | 35 +++++++++++++++++++++-----------
 drivers/md/dm-integrity.c     |  2 +-
 drivers/md/dm-kcopyd.c        | 19 +++++++++++++-----
 drivers/md/dm-linear.c        |  2 +-
 drivers/md/dm-mpath.c         |  6 ++++--
 drivers/md/dm-raid.c          |  3 +--
 drivers/md/dm-raid1.c         |  3 ++-
 drivers/md/dm-rq.c            | 18 ++++++-----------
 drivers/md/dm-snap.c          | 22 +++++++++++++++++++++
 drivers/md/dm-table.c         |  3 +++
 drivers/md/dm-unstripe.c      |  2 +-
 drivers/md/dm-verity-target.c |  9 +++++++++
 drivers/md/dm-writecache.c    |  2 +-
 drivers/md/dm.c               | 46 ++++++++++++-------------------------------
 17 files changed, 121 insertions(+), 82 deletions(-)

Re: [git pull] device mapper changes for 4.21

[pr-tracker-bot]

The pull request you sent on Thu, 27 Dec 2018 11:09:44 -0500:

> git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git tags/for-4.21/dm-changes

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/4ed7bdc1eb4c82cf4bfdf6a94dd36fd695f6f387

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker

Re: [git pull] device mapper changes for 4.21

[pr-tracker-bot]

The pull request you sent on Thu, 27 Dec 2018 11:09:44 -0500:

> git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git tags/for-4.21/dm-changes

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/4ed7bdc1eb4c82cf4bfdf6a94dd36fd695f6f387

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker

Re: [git pull] device mapper changes for 4.21

[Christoph Hellwig]

On Thu, Dec 27, 2018 at 11:09:44AM -0500, Mike Snitzer wrote:
> - Fix various DM targets to check for device sector overflow if
>   CONFIG_LBDAF is not set.

Question to Jens and Linus:

is there any good reason to keep the CONFIG_LBDAF=n option around?
Less than 2gig block devices seem to be an absolutele niche, and I
wonder if it is still worth maintaining the special case just for that.

Re: [git pull] device mapper changes for 4.21

[Christoph Hellwig]

On Thu, Dec 27, 2018 at 11:09:44AM -0500, Mike Snitzer wrote:
> - Fix various DM targets to check for device sector overflow if
>   CONFIG_LBDAF is not set.

Question to Jens and Linus:

is there any good reason to keep the CONFIG_LBDAF=n option around?
Less than 2gig block devices seem to be an absolutele niche, and I
wonder if it is still worth maintaining the special case just for that.

Re: [git pull] device mapper changes for 4.21

[Mikulas Patocka]

On Sun, 30 Dec 2018, Christoph Hellwig wrote:

> On Thu, Dec 27, 2018 at 11:09:44AM -0500, Mike Snitzer wrote:
> > - Fix various DM targets to check for device sector overflow if
> >   CONFIG_LBDAF is not set.
> 
> Question to Jens and Linus:
> 
> is there any good reason to keep the CONFIG_LBDAF=n option around?
> Less than 2gig block devices seem to be an absolutele niche, and I
> wonder if it is still worth maintaining the special case just for that.

The CONFIG_LBDAF limit is 2TiB, not 2GiB.

But you're right that 2TiB devices are common and that perhaps this option 
should go away.

Mikulas

Re: [git pull] device mapper changes for 4.21

[Mikulas Patocka]

On Sun, 30 Dec 2018, Christoph Hellwig wrote:

> On Thu, Dec 27, 2018 at 11:09:44AM -0500, Mike Snitzer wrote:
> > - Fix various DM targets to check for device sector overflow if
> >   CONFIG_LBDAF is not set.
> 
> Question to Jens and Linus:
> 
> is there any good reason to keep the CONFIG_LBDAF=n option around?
> Less than 2gig block devices seem to be an absolutele niche, and I
> wonder if it is still worth maintaining the special case just for that.

The CONFIG_LBDAF limit is 2TiB, not 2GiB.

But you're right that 2TiB devices are common and that perhaps this option 
should go away.

Mikulas

Re: [git pull] device mapper changes for 4.21

[James Bottomley]

On Sun, 2018-12-30 at 01:06 -0800, Christoph Hellwig wrote:
> On Thu, Dec 27, 2018 at 11:09:44AM -0500, Mike Snitzer wrote:
> > - Fix various DM targets to check for device sector overflow if
> >   CONFIG_LBDAF is not set.
> 
> Question to Jens and Linus:
> 
> is there any good reason to keep the CONFIG_LBDAF=n option around?
> Less than 2gig block devices seem to be an absolutele niche, and I
> wonder if it is still worth maintaining the special case just for
> that.

It's really a question for embedded isn't it? since they're the ones
with this set to 'n' in their default configs (linux-arch cc added). 
What LBDAF=n does is make sector_t and blkcnt_t unsigned long instead
of u64.  The maintenance burden to us is we have to use sector_div
instead of do_div in the code and remember that sector_t may be 32 bit
(the current problem in LVM).  The benefit to embedded architectures is
that they don't have to do 64 bit arithmetic for every block
transaction and the price is they can't support block devices larger
than 2TB (not 2GB, although the AF part means we can't support file
sizes bigger than 2GB).

So the first question is: is there an embedded platform where anyone
thinks the cost of the 64 bit arithmetic is important?  and if there is
can they benchmark it so we get an idea of the value of keeping LBDAF
(i.e. if it's just a few percent then likely it's not worth it, if it's
in the tens of percent, it might be).

James

Re: [git pull] device mapper changes for 4.21

[James Bottomley]

On Sun, 2018-12-30 at 01:06 -0800, Christoph Hellwig wrote:
> On Thu, Dec 27, 2018 at 11:09:44AM -0500, Mike Snitzer wrote:
> > - Fix various DM targets to check for device sector overflow if
> >   CONFIG_LBDAF is not set.
> 
> Question to Jens and Linus:
> 
> is there any good reason to keep the CONFIG_LBDAF=n option around?
> Less than 2gig block devices seem to be an absolutele niche, and I
> wonder if it is still worth maintaining the special case just for
> that.

It's really a question for embedded isn't it? since they're the ones
with this set to 'n' in their default configs (linux-arch cc added). 
What LBDAF=n does is make sector_t and blkcnt_t unsigned long instead
of u64.  The maintenance burden to us is we have to use sector_div
instead of do_div in the code and remember that sector_t may be 32 bit
(the current problem in LVM).  The benefit to embedded architectures is
that they don't have to do 64 bit arithmetic for every block
transaction and the price is they can't support block devices larger
than 2TB (not 2GB, although the AF part means we can't support file
sizes bigger than 2GB).

So the first question is: is there an embedded platform where anyone
thinks the cost of the 64 bit arithmetic is important?  and if there is
can they benchmark it so we get an idea of the value of keeping LBDAF
(i.e. if it's just a few percent then likely it's not worth it, if it's
in the tens of percent, it might be).

James

Re: [git pull] device mapper changes for 4.21

[Linus Torvalds]

On Sun, Dec 30, 2018 at 11:15 AM Mikulas Patocka <mpatocka@redhat.com> wrote:
>
> But you're right that 2TiB devices are common and that perhaps this option
> should go away.

2TiB devices are definitely not common in the one situation where this
option might matter: small embedded devices.

I don't think the cost of 64 bit is in the arithmetic, but it might be
in some of the data structures.

But my gut feel is that it probably doesn't much matter, and we could
get rid of the config option without anybody ever noticing. I don't
think we have that many data structures with 'sector_t' in them.

We might try to first just force the option on, and see if anybody even cares.

                  Linus

Re: [git pull] device mapper changes for 4.21

[Linus Torvalds]

On Sun, Dec 30, 2018 at 11:15 AM Mikulas Patocka <mpatocka@redhat.com> wrote:
>
> But you're right that 2TiB devices are common and that perhaps this option
> should go away.

2TiB devices are definitely not common in the one situation where this
option might matter: small embedded devices.

I don't think the cost of 64 bit is in the arithmetic, but it might be
in some of the data structures.

But my gut feel is that it probably doesn't much matter, and we could
get rid of the config option without anybody ever noticing. I don't
think we have that many data structures with 'sector_t' in them.

We might try to first just force the option on, and see if anybody even cares.

                  Linus

Re: [git pull] device mapper changes for 4.21

[Jens Axboe]

On 12/30/18 2:06 AM, Christoph Hellwig wrote:
> On Thu, Dec 27, 2018 at 11:09:44AM -0500, Mike Snitzer wrote:
>> - Fix various DM targets to check for device sector overflow if
>>   CONFIG_LBDAF is not set.
> 
> Question to Jens and Linus:
> 
> is there any good reason to keep the CONFIG_LBDAF=n option around?
> Less than 2gig block devices seem to be an absolutele niche, and I
> wonder if it is still worth maintaining the special case just for that.

I'd be fine with removing it, I seriously doubt that the extended
math is going to be noticeable at all.

I'll try and run some null_blk testing as micro benchmarks when
I'm back in a few days.

-- 
Jens Axboe

Re: [git pull] device mapper changes for 4.21

[Jens Axboe]

On 12/30/18 2:06 AM, Christoph Hellwig wrote:
> On Thu, Dec 27, 2018 at 11:09:44AM -0500, Mike Snitzer wrote:
>> - Fix various DM targets to check for device sector overflow if
>>   CONFIG_LBDAF is not set.
> 
> Question to Jens and Linus:
> 
> is there any good reason to keep the CONFIG_LBDAF=n option around?
> Less than 2gig block devices seem to be an absolutele niche, and I
> wonder if it is still worth maintaining the special case just for that.

I'd be fine with removing it, I seriously doubt that the extended
math is going to be noticeable at all.

I'll try and run some null_blk testing as micro benchmarks when
I'm back in a few days.

-- 
Jens Axboe

Re: [git pull] device mapper changes for 4.21

[Christoph Hellwig]

On Sun, Dec 30, 2018 at 04:25:46PM -0800, Linus Torvalds wrote:
> On Sun, Dec 30, 2018 at 11:15 AM Mikulas Patocka <mpatocka@redhat.com> wrote:
> >
> > But you're right that 2TiB devices are common and that perhaps this option
> > should go away.
> 
> 2TiB devices are definitely not common in the one situation where this
> option might matter: small embedded devices.
> 
> I don't think the cost of 64 bit is in the arithmetic, but it might be
> in some of the data structures.
> 
> But my gut feel is that it probably doesn't much matter, and we could
> get rid of the config option without anybody ever noticing. I don't
> think we have that many data structures with 'sector_t' in them.
> 
> We might try to first just force the option on, and see if anybody even cares.

Our smallest embedded devices use raw flash using the MTD subsystem,
and even that is using 64-bit size types everywhere.  So I'd be really
surprised if it is an issue.

> 
>                   Linus
---end quoted text---

Re: [git pull] device mapper changes for 4.21

[Christoph Hellwig]

On Sun, Dec 30, 2018 at 04:25:46PM -0800, Linus Torvalds wrote:
> On Sun, Dec 30, 2018 at 11:15 AM Mikulas Patocka <mpatocka@redhat.com> wrote:
> >
> > But you're right that 2TiB devices are common and that perhaps this option
> > should go away.
> 
> 2TiB devices are definitely not common in the one situation where this
> option might matter: small embedded devices.
> 
> I don't think the cost of 64 bit is in the arithmetic, but it might be
> in some of the data structures.
> 
> But my gut feel is that it probably doesn't much matter, and we could
> get rid of the config option without anybody ever noticing. I don't
> think we have that many data structures with 'sector_t' in them.
> 
> We might try to first just force the option on, and see if anybody even cares.

Our smallest embedded devices use raw flash using the MTD subsystem,
and even that is using 64-bit size types everywhere.  So I'd be really
surprised if it is an issue.

> 
>                   Linus
---end quoted text---

Re: [git pull] device mapper changes for 4.21

[Milan Broz]

On 03/01/2019 08:27, Christoph Hellwig wrote:
> On Sun, Dec 30, 2018 at 04:25:46PM -0800, Linus Torvalds wrote:
>> On Sun, Dec 30, 2018 at 11:15 AM Mikulas Patocka <mpatocka@redhat.com> wrote:
>>>
>>> But you're right that 2TiB devices are common and that perhaps this option
>>> should go away.
>>
>> 2TiB devices are definitely not common in the one situation where this
>> option might matter: small embedded devices.
>>
>> I don't think the cost of 64 bit is in the arithmetic, but it might be
>> in some of the data structures.
>>
>> But my gut feel is that it probably doesn't much matter, and we could
>> get rid of the config option without anybody ever noticing. I don't
>> think we have that many data structures with 'sector_t' in them.
>>
>> We might try to first just force the option on, and see if anybody even cares.
> 
> Our smallest embedded devices use raw flash using the MTD subsystem,
> and even that is using 64-bit size types everywhere.  So I'd be really
> surprised if it is an issue.

I agree with Christoph here.

(I fixed some CONFIG_LBDAF problems in DM in this pull request because
the code was apparently wrong, but it was a pain to see all these possible
sector overflow checks...)

If it helps anything, we require 64-bit calculations for cryptsetup userspace for >5 years
(you cannot compile it with 32bit support; everything uses 64bit, including these
DM table sector calculations for kernel) and NOBODY complained since we enforced it.
(Ok, it is not a hot path, but....)

Please, if possible, go with 64-bit sector size types by default in future.

Thanks,
Milan

Re: [git pull] device mapper changes for 4.21

[Milan Broz]

On 03/01/2019 08:27, Christoph Hellwig wrote:
> On Sun, Dec 30, 2018 at 04:25:46PM -0800, Linus Torvalds wrote:
>> On Sun, Dec 30, 2018 at 11:15 AM Mikulas Patocka <mpatocka@redhat.com> wrote:
>>>
>>> But you're right that 2TiB devices are common and that perhaps this option
>>> should go away.
>>
>> 2TiB devices are definitely not common in the one situation where this
>> option might matter: small embedded devices.
>>
>> I don't think the cost of 64 bit is in the arithmetic, but it might be
>> in some of the data structures.
>>
>> But my gut feel is that it probably doesn't much matter, and we could
>> get rid of the config option without anybody ever noticing. I don't
>> think we have that many data structures with 'sector_t' in them.
>>
>> We might try to first just force the option on, and see if anybody even cares.
> 
> Our smallest embedded devices use raw flash using the MTD subsystem,
> and even that is using 64-bit size types everywhere.  So I'd be really
> surprised if it is an issue.

I agree with Christoph here.

(I fixed some CONFIG_LBDAF problems in DM in this pull request because
the code was apparently wrong, but it was a pain to see all these possible
sector overflow checks...)

If it helps anything, we require 64-bit calculations for cryptsetup userspace for >5 years
(you cannot compile it with 32bit support; everything uses 64bit, including these
DM table sector calculations for kernel) and NOBODY complained since we enforced it.
(Ok, it is not a hot path, but....)

Please, if possible, go with 64-bit sector size types by default in future.

Thanks,
Milan