| Message ID | 20200304150257.GA19885@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [git,pull] device mapper fixes for 5.6-rc5 | expand |
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
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!
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
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
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
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(-)