Python bindings to `cuFileDriverOpen()` and `cuFileDriverClose()` #514

madsbk · 2024-10-24T13:03:59Z

Changes:

Adding Python bindings to cuFileDriverOpen() and cuFileDriverClose().
We now only open the cufile driver explicitly in CUDA versions older than v12.2.
Introducing kvikio.cufile_driver.initialize(), which open the cuFile driver and close it again at module exit.
Let CI fail if KvikIO wasn't built with cuFile support.
- Except on cuda11.8+arm64; cuFile didn't support arm until cuda v12.4.
Some refactor and clean up!

madsbk · 2024-10-26T18:36:58Z

@EricKern calling kvikio.cufile_driver.initialize() should fix your segfault. Are you able to build KvikIO in your setup? If so, could you test this PR?

EricKern · 2024-10-26T19:02:06Z

Thank you @madsbk for your efforts!
I'll try to build it next week. Maybe I can even find time tomorrow.

We want to trigger CI error if a package was built without cufile support

EricKern · 2024-10-27T10:31:33Z

I've built and reran my small segfault reproducer script without explicitly opening and closing the driver.
This still causes the segfault when I set profile.cufile_stats in cufile.json to anything above 0.
Also when I explicitly open and close the driver it still happens.

If profile.cufile_stats=0 everything works fine.

I guess my segfault (#497) is unrelated to the driver initialization and destruction.

I have tested this on my local machine where I currently don't have a GDS-supported file system. So no actual writing happened.
Only initialization and then cufile's switch to its own compatibility mode.
But even then, the segfault was reproducible on another machine.

madsbk · 2024-10-28T06:58:44Z

Thanks @EricKern, good to get this confirmed. Let's continue the discussion in #497

wence- · 2024-10-28T09:42:48Z

python/kvikio/kvikio/cufile_driver.py

+    return cufile_driver.driver_close()
+
+
+def initialize() -> None:


question: Whose job is it to call initialize?

The user for now. If we find a Python example that segfaults because of cuFile's termination issues, we should consider calling it in __init__.py.

wence-

Tiny suggestions, none are blocking, I think

cpp/include/kvikio/shim/cufile.hpp

wence- · 2024-10-30T09:55:35Z

cpp/include/kvikio/shim/cufile.hpp

+    if (!stream_available) {  // The stream API was introduced in CUDA 12.2.
+      driver_open();


Can we gate this behind the thing we actually mean cuda < 12? It seems we could use something like:

#if CUDA_VERSION_LT(12, 0) driver_open(); #endif

Using a hypothetical macro.

Actually, the version of CTK and cuFile might not match, updated the comment:

// cuFile is supposed to open and close the driver automatically but // because of a bug in cuFile v1.4 (CUDA v11.8), it sometimes segfault: // <https://github.com/rapidsai/kvikio/issues/159>. // We use the stream API as an version indicator of cuFile, it was introduced // in cuFile v1.7 (CUDA v12.2).

wence- · 2024-10-30T09:55:54Z

cpp/include/kvikio/shim/cufile.hpp

-      std::cerr << "Unable to close GDS file driver: " << cufileop_status_error(error.err)
-                << std::endl;
-    }
+    if (!stream_available) { driver_close(); }


wence- · 2024-10-30T09:56:54Z

cpp/include/kvikio/shim/cufile.hpp

+   * cuFile accept multiple calls to `cufileDriverOpen()`, only the first call opens
+   * the driver, but every call should have a matching call to `cufileDriverClose()`.
+   */
+  void driver_open()


question/doc: Should we note that it should be unnecessary to call these functions and that this is a workaround for 11.8 bug? Or does it turn out because the automatic open/close that cufile implements is wrong we do need to do this manually?

It is used by kvikio::DriverInitializer, which can be used to pay for init overhead up front

wence- · 2024-10-30T09:58:10Z

python/kvikio/kvikio/cufile_driver.py

+from kvikio._lib import cufile_driver  # type: ignore
+
+# TODO: Wrap nicely, maybe as a dataclass?
+DriverProperties = cufile_driver.DriverProperties


nit: Add a short docstring?

""" What are the DriverProperties? """

defer to #526

python/kvikio/kvikio/cufile_driver.py

…o cufile_driver

Co-authored-by: Lawrence Mitchell <[email protected]>

madsbk added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Oct 24, 2024

madsbk force-pushed the cufile_driver branch 2 times, most recently from 47a0452 to 4b8181c Compare October 24, 2024 13:10

madsbk mentioned this pull request Oct 24, 2024

kvikio still segfaults on program termination #497

Open

madsbk force-pushed the cufile_driver branch 10 times, most recently from 4967be7 to e7bde28 Compare October 25, 2024 11:50

move to subfolder: cufile

6cfefb8

madsbk force-pushed the cufile_driver branch 2 times, most recently from 3d02555 to 0b4537f Compare October 26, 2024 11:00

cuFileAPI: impl. driver_open() and driver_close()

2e4f50e

madsbk force-pushed the cufile_driver branch from 0b4537f to 2e4f50e Compare October 26, 2024 11:02

madsbk added 3 commits October 26, 2024 15:34

shim: only open the driver on older CUDA installations (<12.2)

60c1deb

rename: driver_properties.pyx => cufile_driver.pyx

4e9edc0

driver open/close python bindings

e1637d7

madsbk force-pushed the cufile_driver branch from 6f7781d to e1637d7 Compare October 26, 2024 14:45

madsbk added 2 commits October 27, 2024 08:44

CI: test whweel use --verbose

edcbe60

Enable test_open_and_close always

032c326

We want to trigger CI error if a package was built without cufile support

madsbk force-pushed the cufile_driver branch from c79f9bb to 032c326 Compare October 27, 2024 07:59

adding a pytest mark "cufile" for tests that requires cufile

77826ee

madsbk force-pushed the cufile_driver branch from 9b28b43 to b43c7ef Compare October 27, 2024 10:02

madsbk force-pushed the cufile_driver branch 5 times, most recently from 37d318f to 3f8f8b7 Compare October 27, 2024 13:51

CI: not run "cufile" marked tests on arm64+cuda11.8

b61ebff

madsbk force-pushed the cufile_driver branch from 3f8f8b7 to b61ebff Compare October 27, 2024 14:25

madsbk marked this pull request as ready for review October 28, 2024 06:59

madsbk requested review from a team as code owners October 28, 2024 06:59

madsbk requested a review from KyleFromNVIDIA October 28, 2024 06:59

wence- reviewed Oct 28, 2024

View reviewed changes

Merge branch 'branch-24.12' into cufile_driver

2efb02e

wence- approved these changes Oct 30, 2024

View reviewed changes

madsbk and others added 5 commits October 30, 2024 11:13

Merge branch 'branch-24.12' of https://github.com/rapidsai/kvikio int…

f0694be

…o cufile_driver

rename _Driver* => Driver*

ee71aac

clean up DriverProperties

2fb04a6

Update cpp/include/kvikio/shim/cufile.hpp

51e6805

Co-authored-by: Lawrence Mitchell <[email protected]>

Update python/kvikio/kvikio/cufile_driver.py

c55c5af

Co-authored-by: Lawrence Mitchell <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Python bindings to `cuFileDriverOpen()` and `cuFileDriverClose()` #514

Python bindings to `cuFileDriverOpen()` and `cuFileDriverClose()` #514

madsbk commented Oct 24, 2024 •

edited

Loading

madsbk commented Oct 26, 2024 •

edited

Loading

EricKern commented Oct 26, 2024

EricKern commented Oct 27, 2024

madsbk commented Oct 28, 2024

wence- Oct 28, 2024

madsbk Oct 28, 2024

wence- left a comment

wence- Oct 30, 2024

madsbk Oct 30, 2024

wence- Oct 30, 2024

wence- Oct 30, 2024

madsbk Oct 30, 2024

wence- Oct 30, 2024

madsbk Oct 30, 2024

		return cufile_driver.driver_close()


		def initialize() -> None:

		if (!stream_available) { // The stream API was introduced in CUDA 12.2.
		driver_open();

Python bindings to cuFileDriverOpen() and cuFileDriverClose() #514

Are you sure you want to change the base?

Python bindings to cuFileDriverOpen() and cuFileDriverClose() #514

Conversation

madsbk commented Oct 24, 2024 • edited Loading

madsbk commented Oct 26, 2024 • edited Loading

EricKern commented Oct 26, 2024

EricKern commented Oct 27, 2024

madsbk commented Oct 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wence- left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Python bindings to `cuFileDriverOpen()` and `cuFileDriverClose()` #514

Python bindings to `cuFileDriverOpen()` and `cuFileDriverClose()` #514

madsbk commented Oct 24, 2024 •

edited

Loading

madsbk commented Oct 26, 2024 •

edited

Loading