Uploaded image for project: 'Lustre'
  1. Lustre
  2. LU-14712

make TRIM state persistent across reboots

Details

    • 9223372036854775807

    Description

      The current "-o discard" mount option for ldiskfs enables on-the-fly TRIM of underlying flash devices (or thinly-provisioned LUNs). However, the current implementation hurts performance because it is done synchronously in the context of the JBD2 commit thread as a commit callback, which blocks later transaction commits.

      There are patches by Shilong that make the TRIM state for a block group persistent, so that running TRIM with mke2fs does not also lead to fstrim resubmitting TRIM requests for all of the groups again immediately after mount/remount.

      Most recent e2fsprogs patches at:
      https://patchwork.ozlabs.org/project/linux-ext4/patch/1590588525-29669-1-git-send-email-wangshilong1991@gmail.com/
      https://patchwork.ozlabs.org/project/linux-ext4/patch/1590588525-29669-2-git-send-email-wangshilong1991@gmail.com/

      ext4 kernel patch:
      https://patchwork.ozlabs.org/project/linux-ext4/patch/1592831677-13945-1-git-send-email-wangshilong1991@gmail.com/
      https://patchwork.ozlabs.org/project/linux-ext4/patch/1592835419-7841-1-git-send-email-wangshilong1991@gmail.com/

      I think the combination of these two patches would improve ongoing ldiskfs performance on flash devices significantly.

      Attachments

        Issue Links

          Activity

            [LU-14712] make TRIM state persistent across reboots

            "Li Dongyang <dongyangli@ddn.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/59312
            Subject: LU-14712 ldiskfs: rename EXT2_FLAGS_TRACK_TRIM to EXT2_FLAGS_NO_TRIM_TRACKING
            Project: fs/lustre-release
            Branch: master
            Current Patch Set: 1
            Commit: d8993774eeae80ffa3b31e7c8ab00654dadfdf9c

            gerrit Gerrit Updater added a comment - "Li Dongyang <dongyangli@ddn.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/59312 Subject: LU-14712 ldiskfs: rename EXT2_FLAGS_TRACK_TRIM to EXT2_FLAGS_NO_TRIM_TRACKING Project: fs/lustre-release Branch: master Current Patch Set: 1 Commit: d8993774eeae80ffa3b31e7c8ab00654dadfdf9c
            dongyang Dongyang Li added a comment -

            Yes indeed we should use EXT2_BG_TRIMMED, the commit message and the debian/changelog wording should be fixed in the next release.

            dongyang Dongyang Li added a comment - Yes indeed we should use EXT2_BG_TRIMMED, the commit message and the debian/changelog wording should be fixed in the next release.

            Sorry, you are right. I was on the wrong branch and "git grep BG_TRIM" did not return anything. I see it now.

            I also noticed it is also a bit confusing that debian/changelog reports "e2fsprogs: support EXT2_FLAG_BG_TRIMMED and EXT2_FLAGS_TRACK_TRIM" instead of EXT2_BG_TRIMMED that the flag is actually named.

            I wanted to check whether doing the "full device discard" during mke2fs would mark all of the groups with EXT2_BG_TRIMMED. It looks like this is the case:

                            retval_discard = io_channel_discard(fs->io, start, count);
                            if (!retval_discard) {
                                    ext2fs_bg_flags_set(fs, group, EXT2_BG_TRIMMED);
                                    ext2fs_group_desc_csum_set(fs, group);
                                    fs->super->s_flags |= EXT2_FLAGS_TRACK_TRIM;
            
            adilger Andreas Dilger added a comment - Sorry, you are right. I was on the wrong branch and " git grep BG_TRIM " did not return anything. I see it now. I also noticed it is also a bit confusing that debian/changelog reports " e2fsprogs: support EXT2_FLAG_BG_TRIMMED and EXT2_FLAGS_TRACK_TRIM " instead of EXT2_BG_TRIMMED that the flag is actually named. I wanted to check whether doing the "full device discard" during mke2fs would mark all of the groups with EXT2_BG_TRIMMED . It looks like this is the case: retval_discard = io_channel_discard(fs->io, start, count); if (!retval_discard) { ext2fs_bg_flags_set(fs, group, EXT2_BG_TRIMMED); ext2fs_group_desc_csum_set(fs, group); fs-> super ->s_flags |= EXT2_FLAGS_TRACK_TRIM;

            dongyang I just noticed that we do not have the patches to support the EXT4_BG_TRIMMED flag in the Lustre e2fsprogs release. Could you please include those patches in the next release.

            adilger Andreas Dilger added a comment - dongyang I just noticed that we do not have the patches to support the EXT4_BG_TRIMMED flag in the Lustre e2fsprogs release. Could you please include those patches in the next release.
            pjones Peter Jones added a comment -

            Merged for 2.16

            pjones Peter Jones added a comment - Merged for 2.16

            "Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/55567/
            Subject: LU-14712 ldiskfs: add bg_trimmed_threshold interface
            Project: fs/lustre-release
            Branch: master
            Current Patch Set:
            Commit: 7ac01793a3d451700fee3be6b655c4494ee158aa

            gerrit Gerrit Updater added a comment - "Oleg Drokin <green@whamcloud.com>" merged in patch https://review.whamcloud.com/c/fs/lustre-release/+/55567/ Subject: LU-14712 ldiskfs: add bg_trimmed_threshold interface Project: fs/lustre-release Branch: master Current Patch Set: Commit: 7ac01793a3d451700fee3be6b655c4494ee158aa
            pjones Peter Jones added a comment -

            Seems to be another patch tracked under this ticket

            pjones Peter Jones added a comment - Seems to be another patch tracked under this ticket

            "Li Dongyang <dongyangli@ddn.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/55567
            Subject: LU-14712 ldiskfs: add bg_trimmed_threshold interface
            Project: fs/lustre-release
            Branch: master
            Current Patch Set: 1
            Commit: 7222197b34c4f2b9dc87805bc3806279041c8fe5

            gerrit Gerrit Updater added a comment - "Li Dongyang <dongyangli@ddn.com>" uploaded a new patch: https://review.whamcloud.com/c/fs/lustre-release/+/55567 Subject: LU-14712 ldiskfs: add bg_trimmed_threshold interface Project: fs/lustre-release Branch: master Current Patch Set: 1 Commit: 7222197b34c4f2b9dc87805bc3806279041c8fe5

            nathand I've created LU-17980 to track the additional work to integrate "-o discard" with the persistent EXT4_BG_TRIMMED flag.

            Despite repeated attempts, we haven't been able to convince the upstream ext4 maintainers to accept those patches, so we will carry them in the ldiskfs patch series for the near future.

            adilger Andreas Dilger added a comment - nathand I've created LU-17980 to track the additional work to integrate " -o discard " with the persistent EXT4_BG_TRIMMED flag. Despite repeated attempts, we haven't been able to convince the upstream ext4 maintainers to accept those patches, so we will carry them in the ldiskfs patch series for the near future.

            pjones it appears that the landed patches only address the persistent flagging of blocks as trimmed. I don't see updates on the more interesting "-o discard" performance improvements from async trimming on a block basis. If there are references to that work, can they be added here, or the ticket reopened for tracking the future work?

            nathand Nathan Dauchy added a comment - pjones it appears that the landed patches only address the persistent flagging of blocks as trimmed. I don't see updates on the more interesting "-o discard" performance improvements from async trimming on a block basis. If there are references to that work, can they be added here, or the ticket reopened for tracking the future work?
            pjones Peter Jones added a comment -

            Merged for 2.16

            pjones Peter Jones added a comment - Merged for 2.16

            People

              dongyang Dongyang Li
              adilger Andreas Dilger
              Votes:
              2 Vote for this issue
              Watchers:
              16 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: