Details

    • Improvement
    • Resolution: Fixed
    • Minor
    • Lustre 2.5.0
    • None
    • 6690

    Description

      Commit 43603bd1 (LU-1548 osd: move i_htree_lock to iam container) eliminated the use of dynlocks inside ldiskfs. Now that it's only used in the system-agnostic mdd and osd-ldiskfs code, we can move it under lustre/ and eliminate a patch we apply during build.

      Attachments

        Issue Links

          Activity

            [LU-2757] Move dynlocks out of ldiskfs

            This patch has just landed. Peter you can close this ticket.

            simmonsja James A Simmons added a comment - This patch has just landed. Peter you can close this ticket.
            adilger Andreas Dilger added a comment - http://review.whamcloud.com/5282

            Updated the patch Alex as you suggested. Now that 2.4 is frozen we can label this a 2.5 item

            simmonsja James A Simmons added a comment - Updated the patch Alex as you suggested. Now that 2.4 is frozen we can label this a 2.5 item

            Ok, it makes sense to add it to an already existing dependency of both oxd-ldiskfs and mdd. I'll update the patch to move it to obdclass.

            jeffm Jeff Mahoney (Inactive) added a comment - Ok, it makes sense to add it to an already existing dependency of both oxd-ldiskfs and mdd. I'll update the patch to move it to obdclass.

            Yuck. Where this code goes depends on if the mdd layer will ever use dynlock. If it wouldn't then it can be moved to osd-ldiskfs. If dynlock will be used with mdd then I suggest it go into the obdclass.

            simmonsja James A Simmons added a comment - Yuck. Where this code goes depends on if the mdd layer will ever use dynlock. If it wouldn't then it can be moved to osd-ldiskfs. If dynlock will be used with mdd then I suggest it go into the obdclass.

            If dynlocks is going to be a whole new kernel module as proposed in the patch, then it will need some work to be dealt with properly in various tools like 'lctl modules' and lustre_rmmod.

            bogl Bob Glossman (Inactive) added a comment - If dynlocks is going to be a whole new kernel module as proposed in the patch, then it will need some work to be dealt with properly in various tools like 'lctl modules' and lustre_rmmod.

            I think that should probably be:

            --- a/lustre/dynlocks/Makefile.in
            +++ b/lustre/dynlocks/Makefile.in
            @@ -2,7 +2,7 @@ all: modules
             install: modules_install
             distdir:
             
            -EXTRA_CFLAGS += -include @abs_top_builddir@/config.h
            +EXTRA_CFLAGS += -include @abs_top_builddir@/config.h -I@abs_srcdir@
             
             sources := dynlocks.c dynlocks.h
            
            jeffm Jeff Mahoney (Inactive) added a comment - I think that should probably be: --- a/lustre/dynlocks/Makefile.in +++ b/lustre/dynlocks/Makefile.in @@ -2,7 +2,7 @@ all: modules install: modules_install distdir: -EXTRA_CFLAGS += -include @abs_top_builddir@/config.h +EXTRA_CFLAGS += -include @abs_top_builddir@/config.h -I@abs_srcdir@ sources := dynlocks.c dynlocks.h

            The 1 line fix below seems to fix it for me.

            --- a/lustre/dynlocks/Makefile.in
            +++ b/lustre/dynlocks/Makefile.in
            @@ -2,7 +2,7 @@ all: modules
             install: modules_install
             distdir:
             
            -EXTRA_CFLAGS += -include @abs_top_builddir@/config.h
            +EXTRA_CFLAGS += -include @abs_top_builddir@/config.h -I@abs_top_builddir@/lustre/dynlocks
             
             sources := dynlocks.c dynlocks.h
            
            
            bogl Bob Glossman (Inactive) added a comment - The 1 line fix below seems to fix it for me. --- a/lustre/dynlocks/Makefile.in +++ b/lustre/dynlocks/Makefile.in @@ -2,7 +2,7 @@ all: modules install: modules_install distdir: -EXTRA_CFLAGS += -include @abs_top_builddir@/config.h +EXTRA_CFLAGS += -include @abs_top_builddir@/config.h -I@abs_top_builddir@/lustre/dynlocks sources := dynlocks.c dynlocks.h

            My builds succeed but you're right, there should be a -I in there.

            jeffm Jeff Mahoney (Inactive) added a comment - My builds succeed but you're right, there should be a -I in there.

            Is it just me or is build of dynlocks.c failing to find dynlocks.h ?
            Seems to be missing a -I in new lustre/dynlocks/Makefile.in

            bogl Bob Glossman (Inactive) added a comment - Is it just me or is build of dynlocks.c failing to find dynlocks.h ? Seems to be missing a -I in new lustre/dynlocks/Makefile.in

            People

              bogl Bob Glossman (Inactive)
              jeffm Jeff Mahoney (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: