mbox series

[git,pull] device mapper fixes for 5.6-rc5

Message ID 20200304150257.GA19885@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: Mike Snitzer
Headers show
Series [git,pull] device mapper fixes for 5.6-rc5 | expand

Pull-request

git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git tags/for-5.6/dm-fixes

Message

Mike Snitzer March 4, 2020, 3:02 p.m. UTC
Hi Linus,

The following changes since commit 47ace7e012b9f7ad71d43ac9063d335ea3d6820b:

  dm: fix potential for q->make_request_fn NULL pointer (2020-01-27 14:52:36 -0500)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git tags/for-5.6/dm-fixes

for you to fetch changes up to 636be4241bdd88fec273b38723e44bad4e1c4fae:

  dm: bump version of core and various targets (2020-03-03 11:10:21 -0500)

Please pull, thanks!
Mike

----------------------------------------------------------------
- Fix request-based DM's congestion_fn and actually wire it up to the
  bdi.

- Extend dm-bio-record to track additional struct bio members needed
  by DM integrity target.

- Fix DM core to properly advertise that a device is suspended during
  unload (between the presuspend and postsuspend hooks).  This change
  is a prereq for related DM integrity and DM writecache fixes.  It
  elevates DM integrity's 'suspending' state tracking to DM core.

- Four stable fixes for DM integrity target.

- Fix crash in DM cache target due to incorrect work item cancelling.

- Fix DM thin metadata lockdep warning that was introduced during 5.6
  merge window.

- Fix DM zoned target's chunk work refcounting that regressed during
  recent conversion to refcount_t.

- Bump the minor version for DM core and all target versions that have
  seen interface changes or important fixes during the 5.6 cycle.

----------------------------------------------------------------
Hou Tao (1):
      dm: fix congested_fn for request-based device

Mike Snitzer (3):
      dm bio record: save/restore bi_end_io and bi_integrity
      dm integrity: use dm_bio_record and dm_bio_restore
      dm: bump version of core and various targets

Mikulas Patocka (6):
      dm integrity: fix recalculation when moving from journal mode to bitmap mode
      dm integrity: fix a deadlock due to offloading to an incorrect workqueue
      dm integrity: fix invalid table returned due to argument count mismatch
      dm cache: fix a crash due to incorrect work item cancelling
      dm: report suspended device during destroy
      dm writecache: verify watermark during resume

Shin'ichiro Kawasaki (1):
      dm zoned: Fix reference counter initial value of chunk works

Theodore Ts'o (1):
      dm thin metadata: fix lockdep complaint

 drivers/md/dm-bio-record.h    | 15 ++++++++
 drivers/md/dm-cache-target.c  |  6 ++--
 drivers/md/dm-integrity.c     | 84 ++++++++++++++++++++++---------------------
 drivers/md/dm-mpath.c         |  2 +-
 drivers/md/dm-thin-metadata.c |  2 +-
 drivers/md/dm-verity-target.c |  2 +-
 drivers/md/dm-writecache.c    | 16 ++++++---
 drivers/md/dm-zoned-target.c  | 10 +++---
 drivers/md/dm.c               | 22 ++++++------
 include/uapi/linux/dm-ioctl.h |  4 +--
 10 files changed, 94 insertions(+), 69 deletions(-)

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Comments

Linus Torvalds March 4, 2020, 7:06 p.m. UTC | #1
On Wed, Mar 4, 2020 at 9:03 AM Mike Snitzer <snitzer@redhat.com> wrote:
>
> - Bump the minor version for DM core and all target versions that have
>   seen interface changes or important fixes during the 5.6 cycle.

Can we please remove these pointless version markers entirely?

They make no sense. The kernel doesn't allow backwards incompatible
changes anyway, so the whole point of using some kind of interface
versioning is entirely bogus.

The way you test if a new feature exists or not is to just use it, and
if you're running on an old kernel that doesn't support that
operation, then it should return an error.

             Linus


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
pr-tracker-bot@kernel.org March 4, 2020, 7:15 p.m. UTC | #2
The pull request you sent on Wed, 4 Mar 2020 10:02:57 -0500:

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

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/776e49e8ddb5169e6477fd33a396e9c7b2eb7400

Thank you!
Mike Snitzer March 4, 2020, 7:23 p.m. UTC | #3
On Wed, Mar 04 2020 at  2:06pm -0500,
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Wed, Mar 4, 2020 at 9:03 AM Mike Snitzer <snitzer@redhat.com> wrote:
> >
> > - Bump the minor version for DM core and all target versions that have
> >   seen interface changes or important fixes during the 5.6 cycle.
> 
> Can we please remove these pointless version markers entirely?
> 
> They make no sense. The kernel doesn't allow backwards incompatible
> changes anyway, so the whole point of using some kind of interface
> versioning is entirely bogus.
> 
> The way you test if a new feature exists or not is to just use it, and
> if you're running on an old kernel that doesn't support that
> operation, then it should return an error.

These versions are for userspace's benefit (be it lvm2, cryptsetup,
multipath-tools, etc).  But yes, these versions are bogus even for
that -- primarily because it requires userspace to know when a
particular feature/fix it cares about was introduced.  In addition: if
fixes, that also bump version, are marked for stable@ then we're quickly
in versioning hell -- which is why I always try to decouple version
bumps from fixes.

Others have suggested setting feature flags.  I expect you'd hate those
too.  I suspect I quickly would too given flag bits are finite and
really tedious to deal with.

I'll think further about this issue and consult with userspace
developers and see what we might do.

Thanks (for the needed kick in the ass).
Mike

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Linus Torvalds March 4, 2020, 7:34 p.m. UTC | #4
On Wed, Mar 4, 2020, 13:23 Mike Snitzer <snitzer@redhat.com> wrote:

>
> These versions are for userspace's benefit (be it lvm2, cryptsetup,
> multipath-tools, etc).  But yes, these versions are bogus even for
> that -- primarily because it requires userspace to know when a
> particular feature/fix it cares about was introduced.  In addition: if
> fixes, that also bump version, are marked for stable@ then we're quickly
> in versioning hell -- which is why I always try to decouple version
> bumps from fixes.
>

Yeah, I think the drm people used to have a version number too, and it's
not just fixes getting backported to stable - it's distro kernels taking
changes for new hardware without taking other parts etc.

So the versioning ends up not ever working reliably anyway - the same way
that you can't use the kernel version number to determine what system calls
are available.

So versions can not ever be anything more than informational, and it's
usually just very confusing to have multiple different version numbers (ie
"I'm running kernel v5.4, and my driver abc version is 1.4.2a" is *not* in
the least helpful).

Others have suggested setting feature flags.  I expect you'd hate those
> too.  I suspect I quickly would too given flag bits are finite and
> really tedious to deal with.
>

It also leads to some people then thinking it's ok to remove features
(perhaps to reimplement them differently) if they only clear the feature
bit.

And no, it's not how kernel interfaces work. We keep the interfaces even if
the internals change.

So I've been suggesting that people just freeze the version, or remove the
interface entirely is possible.

Because otherwise it's just a source of problems, where user space might
refuse to do something that the kernel supports because of some silly
version check...

       Linus

>
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Theodore Ts'o March 4, 2020, 8:02 p.m. UTC | #5
On Wed, Mar 04, 2020 at 02:23:35PM -0500, Mike Snitzer wrote:
> 
> These versions are for userspace's benefit (be it lvm2, cryptsetup,
> multipath-tools, etc).  But yes, these versions are bogus even for
> that -- primarily because it requires userspace to know when a
> particular feature/fix it cares about was introduced.  In addition: if
> fixes, that also bump version, are marked for stable@ then we're quickly
> in versioning hell -- which is why I always try to decouple version
> bumps from fixes.
> 
> Others have suggested setting feature flags.  I expect you'd hate those
> too.  I suspect I quickly would too given flag bits are finite and
> really tedious to deal with.
> 
> I'll think further about this issue and consult with userspace
> developers and see what we might do.

What I do for e2fsprogs is that it looks for the existence of files in
/sys/fs/ext4/features:

% ls /sys/fs/ext4/features/
total 0
0 batched_discard  0 encryption        0 meta_bg_resize      0 verity
0 casefold         0 lazy_itable_init  0 metadata_csum_seed

I started this because sometimes ext4 features get backported
(sometimes to ancient 3.18 kernels for Android, sigh, but also once or
twice for enterprise distro kernels), and so this relieves e2fsprogs
from testing kernel versions when deciding which defaults at mke2fs
time (for example).

						- Ted


--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Zdenek Kabelac March 5, 2020, 9:44 a.m. UTC | #6
Dne 04. 03. 20 v 20:34 Linus Torvalds napsal(a):
> 
> 
> On Wed, Mar 4, 2020, 13:23 Mike Snitzer <snitzer@redhat.com 
> <mailto:snitzer@redhat.com>> wrote:
> 
> 
>     These versions are for userspace's benefit (be it lvm2, cryptsetup,
>     multipath-tools, etc).  But yes, these versions are bogus even for
>     that -- primarily because it requires userspace to know when a
>     particular feature/fix it cares about was introduced.  In addition: if
>     fixes, that also bump version, are marked for stable@ then we're quickly
>     in versioning hell -- which is why I always try to decouple version
>     bumps from fixes.
> 
> 
> Yeah, I think the drm people used to have a version number too, and it's not 
> just fixes getting backported to stable - it's distro kernels taking changes 
> for new hardware without taking other parts etc.
> 
> So the versioning ends up not ever working reliably anyway - the same way that 
> you can't use the kernel version number to determine what system calls are 
> available.
> 
> So versions can not ever be anything more than informational, and it's usually 
> just very confusing to have multiple different version numbers (ie "I'm 
> running kernel v5.4, and my driver abc version is 1.4.2a" is *not* in the 
> least helpful).
> 
>     Others have suggested setting feature flags.  I expect you'd hate those
>     too.  I suspect I quickly would too given flag bits are finite and
>     really tedious to deal with.
> 
> 
> It also leads to some people then thinking it's ok to remove features (perhaps 
> to reimplement them differently) if they only clear the feature bit.
> 
> And no, it's not how kernel interfaces work. We keep the interfaces even if 
> the internals change.
> 
> So I've been suggesting that people just freeze the version, or remove the 
> interface entirely is possible.
> 
> Because otherwise it's just a source of problems, where user space might 
> refuse to do something that the kernel supports because of some silly version 
> check...

Hi

POV of lvm2 developer - there are 2 things to solve - 1st. is the introduction 
of a new 'features'. The 2nd. is usability/stability of certain version of dm 
targets - so when we later discover some combination of device stack are not 
safe to use (can lead to significant lose of user's data) lvm2 adds check for 
this.

The reason for complexity comes from fact - numerous distribution use version
of kernel X.Y.Z while they can have much new DM target version as it's much 
more simple to backport new DM into older version.

Nothing is clearly 'perfect', there is no ideal solution to cover all 
combination of all kernel backports - but current separate versioning stream
of DM targets added to kernel versioning, which i.e. lvm2 also is tracking, 
adds more hints for safe decision (as the safety of user's data is the most 
important here)  and allows various distributions to 'somehow reasonably' 
handle backporting of bugfixes.

So if there would be 'feature flag' list provided by DM target - there still 
should be visible which version of implemented flag is that - as when the new 
feature is added - it's not always 'perfect' - sometimes we discover q bug 
quite late in the process of new feature introduction - so the plain fact 
'featureXYZ' is present unfortunately doesn't always mean it's usable.
Sometimes even 'fixing' one bug may introduce a new problem we discover again 
later (testing combinations of device stacks is really madness of its own...)


Zdenek

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel