libnxz / power-gzip

POWER NX zlib compliant library
23 stars 18 forks source link

Use proper error code to check fault address #39

Closed rzinsly closed 4 years ago

rzinsly commented 4 years ago

The completion codes were fixed on kernel (#38), now ERR_NX_TRANSLATION (CSB.CC=5) is only used for internal VAS faults handling and should not used by libnxz. ERR_NX_AT_FAULT (CSB.CC=250) is the proper error code reported by the kernel when NX encounters address translation failure.

rzinsly commented 4 years ago

Maybe you can remove the definition of ERR_NX_TRANSLATION if its not being used anywhere else.

We have all possible error codes defined, maybe we can remove the ones that we don't use but removing only ERR_NX_TRANSLATION seems wrong.

tuliom commented 4 years ago

It's my understanding this modification will break compatibility with older versions of the kernel patch. Is there a way to make the change and continue to support older kernels?

rzinsly commented 4 years ago

It's my understanding this modification will break compatibility with older versions of the kernel patch. Is there a way to make the change and continue to support older kernels?

I tested with an older kernel and works fine. This doesn't actually break anything, the patch changes error handling, if the compression goes well nothing changes. Applying this patch with older kernel versions if we got an address translation fault the application will stop and print the CC value that caused it instead of printing an warning, treating the error and retrying with fewer pages. The only way I can think of keeping the same behaviour on both older and new kernel versions is by treating CC 5 and CC 250 the same, but if we got the internal CC 5 after the kernel fix something went wrong and for me it seems we should stop the application.