From 7b801a0ce0fca4ab32ed97a13272f0cc57c281fd Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Wed, 31 May 2023 11:16:30 +0200 Subject: [PATCH 1/3] Add option to load NVIDIA kernel modules These changes add a --load-kernel-modules option to the nvidia-ctk system commands. If specified the NVIDIA kernel modules (nvidia, nvidia-uvm, and nvidia-modeset) are loaded before any operations on device nodes are performed. Signed-off-by: Evan Lezar --- .../create-dev-char-symlinks.go | 57 +++++++++++++++---- .../create-device-nodes.go | 8 +++ internal/system/options.go | 7 +++ internal/system/system.go | 35 ++++++++++-- 4 files changed, 92 insertions(+), 15 deletions(-) diff --git a/cmd/nvidia-ctk/system/create-dev-char-symlinks/create-dev-char-symlinks.go b/cmd/nvidia-ctk/system/create-dev-char-symlinks/create-dev-char-symlinks.go index f84734b3..03b9de78 100644 --- a/cmd/nvidia-ctk/system/create-dev-char-symlinks/create-dev-char-symlinks.go +++ b/cmd/nvidia-ctk/system/create-dev-char-symlinks/create-dev-char-symlinks.go @@ -24,6 +24,7 @@ import ( "strings" "syscall" + "github.com/NVIDIA/nvidia-container-toolkit/internal/system" "github.com/fsnotify/fsnotify" "github.com/sirupsen/logrus" "github.com/urfave/cli/v2" @@ -38,11 +39,12 @@ type command struct { } type config struct { - devCharPath string - driverRoot string - dryRun bool - watch bool - createAll bool + devCharPath string + driverRoot string + dryRun bool + watch bool + createAll bool + loadKernelModules bool } // NewCommand constructs a command sub-command with the specified logger @@ -97,6 +99,12 @@ func (m command) build() *cli.Command { Destination: &cfg.createAll, EnvVars: []string{"CREATE_ALL"}, }, + &cli.BoolFlag{ + Name: "load-kernel-modules", + Usage: "Load the NVIDIA kernel modules before creating symlinks. This is only applicable when --create-all is set.", + Destination: &cfg.loadKernelModules, + EnvVars: []string{"LOAD_KERNEL_MODULES"}, + }, &cli.BoolFlag{ Name: "dry-run", Usage: "If set, the command will not create any symlinks.", @@ -114,6 +122,11 @@ func (m command) validateFlags(r *cli.Context, cfg *config) error { return fmt.Errorf("create-all and watch are mutually exclusive") } + if cfg.loadKernelModules && !cfg.createAll { + m.logger.Warn("load-kernel-modules is only applicable when create-all is set; ignoring") + cfg.loadKernelModules = false + } + return nil } @@ -137,6 +150,7 @@ func (m command) run(c *cli.Context, cfg *config) error { WithDriverRoot(cfg.driverRoot), WithDryRun(cfg.dryRun), WithCreateAll(cfg.createAll), + WithLoadKernelModules(cfg.loadKernelModules), ) if err != nil { return fmt.Errorf("failed to create symlink creator: %v", err) @@ -186,12 +200,13 @@ create: } type linkCreator struct { - logger *logrus.Logger - lister nodeLister - driverRoot string - devCharPath string - dryRun bool - createAll bool + logger *logrus.Logger + lister nodeLister + driverRoot string + devCharPath string + dryRun bool + createAll bool + loadKernelModules bool } // Creator is an interface for creating symlinks to /dev/nv* devices in /dev/char. @@ -218,6 +233,19 @@ func NewSymlinkCreator(opts ...Option) (Creator, error) { c.devCharPath = defaultDevCharPath } + if c.loadKernelModules { + s, err := system.New( + system.WithLogger(c.logger), + system.WithDryRun(c.dryRun), + ) + if err != nil { + return nil, err + } + if err := s.LoadNVIDIAKernelModules(); err != nil { + return nil, fmt.Errorf("failed to load NVIDIA kernel modules: %v", err) + } + } + if c.createAll { lister, err := newAllPossible(c.logger, c.driverRoot) if err != nil { @@ -265,6 +293,13 @@ func WithCreateAll(createAll bool) Option { } } +// WithLoadKernelModules sets the loadKernelModules flag for the linkCreator. +func WithLoadKernelModules(loadKernelModules bool) Option { + return func(lc *linkCreator) { + lc.loadKernelModules = loadKernelModules + } +} + // CreateLinks creates symlinks for all NVIDIA device nodes found in the driver root. func (m linkCreator) CreateLinks() error { deviceNodes, err := m.lister.DeviceNodes() diff --git a/cmd/nvidia-ctk/system/create-device-nodes/create-device-nodes.go b/cmd/nvidia-ctk/system/create-device-nodes/create-device-nodes.go index d1fb346e..508177bf 100644 --- a/cmd/nvidia-ctk/system/create-device-nodes/create-device-nodes.go +++ b/cmd/nvidia-ctk/system/create-device-nodes/create-device-nodes.go @@ -34,6 +34,8 @@ type options struct { dryRun bool control bool + + loadKernelModules bool } // NewCommand constructs a command sub-command with the specified logger @@ -72,6 +74,11 @@ func (m command) build() *cli.Command { Usage: "create all control device nodes: nvidiactl, nvidia-modeset, nvidia-uvm, nvidia-uvm-tools", Destination: &opts.control, }, + &cli.BoolFlag{ + Name: "load-kernel-modules", + Usage: "load the NVIDIA Kernel Modules before creating devices nodes", + Destination: &opts.loadKernelModules, + }, &cli.BoolFlag{ Name: "dry-run", Usage: "if set, the command will not create any symlinks.", @@ -92,6 +99,7 @@ func (m command) run(c *cli.Context, opts *options) error { s, err := system.New( system.WithLogger(m.logger), system.WithDryRun(opts.dryRun), + system.WithLoadKernelModules(opts.loadKernelModules), ) if err != nil { return fmt.Errorf("failed to create library: %v", err) diff --git a/internal/system/options.go b/internal/system/options.go index fb0fbb38..de3bf21d 100644 --- a/internal/system/options.go +++ b/internal/system/options.go @@ -34,3 +34,10 @@ func WithDryRun(dryRun bool) Option { i.dryRun = dryRun } } + +// WithLoadKernelModules sets the load kernel modules flag +func WithLoadKernelModules(loadKernelModules bool) Option { + return func(i *Interface) { + i.loadKernelModules = loadKernelModules + } +} diff --git a/internal/system/system.go b/internal/system/system.go index d3ad63eb..fe745160 100644 --- a/internal/system/system.go +++ b/internal/system/system.go @@ -19,6 +19,7 @@ package system import ( "fmt" "os" + "os/exec" "path/filepath" "strings" @@ -29,10 +30,10 @@ import ( // Interface is the interface for the system command type Interface struct { - logger *logrus.Logger - dryRun bool - - nvidiaDevices nvidiaDevices + logger *logrus.Logger + dryRun bool + loadKernelModules bool + nvidiaDevices nvidiaDevices } // New constructs a system command with the specified options @@ -44,6 +45,12 @@ func New(opts ...Option) (*Interface, error) { opt(i) } + if i.loadKernelModules { + if err := i.LoadNVIDIAKernelModules(); err != nil { + return nil, fmt.Errorf("failed to load kernel modules: %v", err) + } + } + devices, err := devices.GetNVIDIADevices() if err != nil { return nil, fmt.Errorf("failed to create devices info: %v", err) @@ -108,6 +115,26 @@ func (m *Interface) createDeviceNode(path string, major int, minor int) error { return unix.Chmod(path, 0666) } +// LoadNVIDIAKernelModules loads the NVIDIA kernel modules. +func (m *Interface) LoadNVIDIAKernelModules() error { + modules := []string{"nvidia", "nvidia-uvm", "nvidia-modeset"} + + for _, module := range modules { + if m.dryRun { + m.logger.Infof("Running: /sbin/modprobe %s", module) + continue + } + cmd := exec.Command("/sbin/modprobe", module) + + if output, err := cmd.CombinedOutput(); err != nil { + m.logger.Debugf("Failed to load kernel module %s: %v", module, string(output)) + return fmt.Errorf("failed to load kernel module %s: %v", module, err) + } + } + + return nil +} + type nvidiaDevices struct { devices.Devices } From b64ba6ac2de8537202396c4efcd3e039565447ce Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Wed, 31 May 2023 11:49:38 +0200 Subject: [PATCH 2/3] Add option to create device nodes This change adds a --create-device-nodes option to the nvidia-ctk system create-dev-char-symlinks command to create device nodes. The currently only creates control device nodes. Signed-off-by: Evan Lezar --- .../create-dev-char-symlinks.go | 62 +++++++++++++++---- 1 file changed, 51 insertions(+), 11 deletions(-) diff --git a/cmd/nvidia-ctk/system/create-dev-char-symlinks/create-dev-char-symlinks.go b/cmd/nvidia-ctk/system/create-dev-char-symlinks/create-dev-char-symlinks.go index 03b9de78..8bd1e563 100644 --- a/cmd/nvidia-ctk/system/create-dev-char-symlinks/create-dev-char-symlinks.go +++ b/cmd/nvidia-ctk/system/create-dev-char-symlinks/create-dev-char-symlinks.go @@ -44,6 +44,7 @@ type config struct { dryRun bool watch bool createAll bool + createDeviceNodes bool loadKernelModules bool } @@ -105,6 +106,12 @@ func (m command) build() *cli.Command { Destination: &cfg.loadKernelModules, EnvVars: []string{"LOAD_KERNEL_MODULES"}, }, + &cli.BoolFlag{ + Name: "create-device-nodes", + Usage: "Create the NVIDIA control device nodes in the driver root if they do not exist. This is only applicable when --create-all is set", + Destination: &cfg.createDeviceNodes, + EnvVars: []string{"CREATE_DEVICE_NODES"}, + }, &cli.BoolFlag{ Name: "dry-run", Usage: "If set, the command will not create any symlinks.", @@ -127,6 +134,11 @@ func (m command) validateFlags(r *cli.Context, cfg *config) error { cfg.loadKernelModules = false } + if cfg.createDeviceNodes && !cfg.createAll { + m.logger.Warn("create-device-nodes is only applicable when create-all is set; ignoring") + cfg.createDeviceNodes = false + } + return nil } @@ -151,6 +163,7 @@ func (m command) run(c *cli.Context, cfg *config) error { WithDryRun(cfg.dryRun), WithCreateAll(cfg.createAll), WithLoadKernelModules(cfg.loadKernelModules), + WithCreateDeviceNodes(cfg.createDeviceNodes), ) if err != nil { return fmt.Errorf("failed to create symlink creator: %v", err) @@ -206,6 +219,7 @@ type linkCreator struct { devCharPath string dryRun bool createAll bool + createDeviceNodes bool loadKernelModules bool } @@ -233,17 +247,8 @@ func NewSymlinkCreator(opts ...Option) (Creator, error) { c.devCharPath = defaultDevCharPath } - if c.loadKernelModules { - s, err := system.New( - system.WithLogger(c.logger), - system.WithDryRun(c.dryRun), - ) - if err != nil { - return nil, err - } - if err := s.LoadNVIDIAKernelModules(); err != nil { - return nil, fmt.Errorf("failed to load NVIDIA kernel modules: %v", err) - } + if err := c.setup(); err != nil { + return nil, err } if c.createAll { @@ -258,6 +263,34 @@ func NewSymlinkCreator(opts ...Option) (Creator, error) { return c, nil } +func (m linkCreator) setup() error { + if !m.loadKernelModules && !m.createDeviceNodes { + return nil + } + + s, err := system.New( + system.WithLogger(m.logger), + system.WithDryRun(m.dryRun), + ) + if err != nil { + return err + } + + if m.loadKernelModules { + if err := s.LoadNVIDIAKernelModules(); err != nil { + return fmt.Errorf("failed to load NVIDIA kernel modules: %v", err) + } + } + + if m.createDeviceNodes { + if err := s.CreateNVIDIAControlDeviceNodesAt(m.driverRoot); err != nil { + return fmt.Errorf("failed to create NVIDIA device nodes: %v", err) + } + } + + return nil +} + // WithDriverRoot sets the driver root path. func WithDriverRoot(root string) Option { return func(c *linkCreator) { @@ -300,6 +333,13 @@ func WithLoadKernelModules(loadKernelModules bool) Option { } } +// WithCreateDeviceNodes sets the createDeviceNodes flag for the linkCreator. +func WithCreateDeviceNodes(createDeviceNodes bool) Option { + return func(lc *linkCreator) { + lc.createDeviceNodes = createDeviceNodes + } +} + // CreateLinks creates symlinks for all NVIDIA device nodes found in the driver root. func (m linkCreator) CreateLinks() error { deviceNodes, err := m.lister.DeviceNodes() From eca13e72bf480c48c2a579fc035278c0f370b6c6 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Wed, 31 May 2023 19:33:31 +0200 Subject: [PATCH 3/3] Update CHANGELOG Signed-off-by: Evan Lezar --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9aaba665..9420f78b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ * Use *.* pattern to locate libcuda.so when generating a CDI specification to support platforms where a patch version is not specified. * Update go-nvlib to skip devices that are not MIG capable when generating CDI specifications. * Add `nvidia-container-runtime-hook.path` config option to specify NVIDIA Container Runtime Hook path explicitly. +* Fix bug in creation of `/dev/char` symlinks by failing operation if kernel modules are not loaded. +* Add option to load kernel modules when creating device nodes +* Add option to create device nodes when creating `/dev/char` symlinks ## v1.13.1