Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Latest kvdo push (6.2.2.117) does not build on 5.6 kernel #29

Closed
rhawalsh opened this issue Feb 26, 2020 · 10 comments
Closed

Latest kvdo push (6.2.2.117) does not build on 5.6 kernel #29

rhawalsh opened this issue Feb 26, 2020 · 10 comments

Comments

@rhawalsh
Copy link
Member

Currently the build is failing due to the time_t symbol being removed in the 5.6 kernel.

make: Entering directory '/usr/src/kernels/5.6.0-0.rc3.git0.1.fc32.x86_64'
  AR      /var/lib/dkms/kvdo/6.2.2.117-6.2.2.117/build/uds/built-in.a
  CC [M]  /var/lib/dkms/kvdo/6.2.2.117-6.2.2.117/build/uds/indexInternals.o
In file included from /var/lib/dkms/kvdo/6.2.2.117-6.2.2.117/build/uds/common.h:27,
                 from /var/lib/dkms/kvdo/6.2.2.117-6.2.2.117/build/uds/openChapterZone.h:25,
                 from /var/lib/dkms/kvdo/6.2.2.117-6.2.2.117/build/uds/chapterWriter.h:27,
                 from /var/lib/dkms/kvdo/6.2.2.117-6.2.2.117/build/uds/index.h:25,
                 from /var/lib/dkms/kvdo/6.2.2.117-6.2.2.117/build/uds/indexInternals.h:25,
                 from /var/lib/dkms/kvdo/6.2.2.117-6.2.2.117/build/uds/indexInternals.c:22:
/var/lib/dkms/kvdo/6.2.2.117-6.2.2.117/build/uds/uds.h:182:3: error: unknown type name ‘time_t’
  182 |   time_t   currentTime;
      |   ^~~~~~
In file included from /var/lib/dkms/kvdo/6.2.2.117-6.2.2.117/build/uds/threads.h:27,
                 from /var/lib/dkms/kvdo/6.2.2.117-6.2.2.117/build/uds/indexSession.h:29,
                 from /var/lib/dkms/kvdo/6.2.2.117-6.2.2.117/build/uds/index.h:27,
                 from /var/lib/dkms/kvdo/6.2.2.117-6.2.2.117/build/uds/indexInternals.h:25,
                 from /var/lib/dkms/kvdo/6.2.2.117-6.2.2.117/build/uds/indexInternals.c:22:
/var/lib/dkms/kvdo/6.2.2.117-6.2.2.117/build/uds/timeUtils.h:225:15: error: unknown type name ‘time_t’
  225 | static INLINE time_t asTimeT(AbsTime time)
      |               ^~~~~~
/var/lib/dkms/kvdo/6.2.2.117-6.2.2.117/build/uds/timeUtils.h:241:33: error: unknown type name ‘time_t’; did you mean ‘ktime_t’?
  241 | static INLINE AbsTime fromTimeT(time_t time)
      |                                 ^~~~~~
      |                                 ktime_t
make[2]: *** [scripts/Makefile.build:268: /var/lib/dkms/kvdo/6.2.2.117-6.2.2.117/build/uds/indexInternals.o] Error 1
make[1]: *** [scripts/Makefile.build:505: /var/lib/dkms/kvdo/6.2.2.117-6.2.2.117/build/uds] Error 2
make: *** [Makefile:1681: /var/lib/dkms/kvdo/6.2.2.117-6.2.2.117/build] Error 2
make: Leaving directory '/usr/src/kernels/5.6.0-0.rc3.git0.1.fc32.x86_64'


@rhawalsh
Copy link
Member Author

This is fixed on my fork [0].

It requires 3 patches. One of those patches (no-vlas)[1] has been needed for over a year at this point. The other two are new as of kernel 5.6 (time_t [2] and procfs [3]).

[0] https://github.com/rhawalsh/kvdo/tree/6.2.2.117-COPR
[1] rhawalsh@ec66fd2
[2] rhawalsh@3823b2b
[3] rhawalsh@ff4c6a2

@MarcoR83
Copy link

why close, when this is still needs a patched version is not part of an release?

@MarcoR83
Copy link

MarcoR83 commented May 18, 2020

Trying to build https://github.com/rhawalsh/kvdo/tree/6.2.2.117-COPR on arch with kernel 5.6.11-arch1-1 gives me the following compile error:

In file included from /var/lib/dkms/kvdo/6.2.2.117/build/vdo/../uds/memoryAlloc.h:30,
from /var/lib/dkms/kvdo/6.2.2.117/build/vdo/kernel/histogram.c:24:
/var/lib/dkms/kvdo/6.2.2.117/build/vdo/kernel/histogram.c: In function ‘makeLogarithmicJiffiesHistogram’:
/var/lib/dkms/kvdo/6.2.2.117/build/vdo/../uds/permassert.h:114:5: error: duplicate case value
114 |     case expr:              \
|     ^~~~
/var/lib/dkms/kvdo/6.2.2.117/build/vdo/kernel/histogram.c:600:3: note: in expansion of macro ‘STATIC_ASSERT’
600 |   STATIC_ASSERT((MSEC_PER_SEC % HZ) == 0);
|   ^~~~~~~~~~~~~
/var/lib/dkms/kvdo/6.2.2.117/build/vdo/../uds/permassert.h:113:5: note: previously used here
113 |     case 0:                 \
|     ^~~~
/var/lib/dkms/kvdo/6.2.2.117/build/vdo/kernel/histogram.c:600:3: note: in expansion of macro ‘STATIC_ASSERT’
600 |   STATIC_ASSERT((MSEC_PER_SEC % HZ) == 0);
|   ^~~~~~~~~~~~~
make[2]: *** [scripts/Makefile.build:268: /var/lib/dkms/kvdo/6.2.2.117/build/vdo/kernel/histogram.o] Error 1

help :-)

@MarcoR83
Copy link

ok I made it compile. I removed two STATIC_ASSERT calls in function makeLogarithmicJiffiesHistogram (histogram.c:600)

@rhawalsh
Copy link
Member Author

Hi @MarcoR83,

Thanks for the report. You should be able to see a comment about this issue addressed in Issue 22. The change you made was suggested as being sufficient and shouldn't have an effect on the operation of the VDO volume.

With regard to the comment about closing this issue. I decided to close since there was a resolution provided. While it's not available in this repo, it will always be updated in my forked repo since we provide this in a COPR repo that requires these kinds of changes to be implemented.

I am going to re-open this issue with a proposed resolution of adding a link to my forked repo for building against the most modern kernels. That is, until we start getting our development upstream kernel candidate code posted automatically (which is in the process of being implemented).

@rhawalsh rhawalsh reopened this May 18, 2020
@MarcoR83
Copy link

MarcoR83 commented May 18, 2020

Thanks for the fast reply. Yes it is working again. Thats the price you have to pay when living on the bleeding edge. ;-) So your fork/repo will always be updated to the newest version of kvdo? Hope to see this awesome project being integrated into mainline in the future :-)

@rhawalsh
Copy link
Member Author

Good to hear that you're able to use the software again.

Yes, since I maintain the COPR repo that is linked, I work to make sure that it contains the patches necessary to work on the relevant kernels for Fedora releases, which is usually pretty close to what is the latest-available for the mainline kernel.

Thank you for the enthusiasm! We're certainly excited to try and get integrated as well. Please do not hesitate to contact us with any additional questions or issues you might have.

@hmoffatt
Copy link

hmoffatt commented Nov 6, 2020

What version can I compile on 5.8? I tried your fork linked above @rhawalsh but it fails with a different error:

/home/hamish/vdo/kvdo/uds/memoryLinuxKernel.c:282:13: error: too many arguments to function ‘__vmalloc’
         p = __vmalloc(size, gfpFlags | __GFP_NOWARN, PAGE_KERNEL);
             ^~~~~~~~~
In file included from /home/hamish/vdo/kvdo/uds/memoryLinuxKernel.c:28:
/usr/src/linux-headers-5.8.0-0.bpo.2-common/include/linux/vmalloc.h:111:14: note: declared here
 extern void *__vmalloc(unsigned long size, gfp_t gfp_mask);
              ^~~~~~~~~
/home/hamish/vdo/kvdo/uds/memoryLinuxKernel.c:291:13: error: too many arguments to function ‘__vmalloc’
         p = __vmalloc(size, gfpFlags, PAGE_KERNEL);
             ^~~~~~~~~
In file included from /home/hamish/vdo/kvdo/uds/memoryLinuxKernel.c:28:
/usr/src/linux-headers-5.8.0-0.bpo.2-common/include/linux/vmalloc.h:111:14: note: declared here
 extern void *__vmalloc(unsigned long size, gfp_t gfp_mask);
              ^~~~~~~~~

@rhawalsh
Copy link
Member Author

rhawalsh commented Nov 6, 2020

@hmoffatt can you please confirm for me that you're pointing at the *-COPR branches? I believe the error you're seeing was first adddressed on my 6.2.3.107-COPR branch.

There should be a -COPR branch for every version that has been tagged in this repository, so anything newer than 6.2.3.107 should have what you need to build.

@hmoffatt
Copy link

hmoffatt commented Nov 8, 2020

Thanks @rhawalsh I have 6.2.4.26-COPR compiled and working now on Linux 5.8.

corwin pushed a commit that referenced this issue Dec 4, 2020
Merged 144340 to linux-vdo.
144340 by awalsh on 2020/05/18 (pair: sclafani)
Updated README for github to mention updated fork.

	Github issue 29 [0] mentioned that it shouldn't have been closed since
	the issue was still present.  The comment that was provided upon
	closing should have been sufficient, but it can still be missed if the
	user isn't specifically searching for it.  Adding a note to the README
	should alleviate this disconnect.

	[0] #29

	.../README.md
	Added a pointer to rhawalsh/kvdo fork.

Co-authored-by: awalsh@redhat.com
Co-authored-by: sclafani@redhat.com
@corwin corwin closed this as completed Mar 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants