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

Add support for lock debugging #18796

Merged
merged 5 commits into from
Jun 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions cmd/podman/system/locks.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package system

import (
"fmt"

"github.com/containers/podman/v4/cmd/podman/registry"
"github.com/containers/podman/v4/cmd/podman/validate"
"github.com/spf13/cobra"
)

var (
locksCommand = &cobra.Command{
Use: "locks",
Short: "Debug Libpod's use of locks, identifying any potential conflicts",
Args: validate.NoArgs,
Hidden: true,
RunE: func(cmd *cobra.Command, args []string) error {
return runLocks()
},
Example: "podman system locks",
}
)

func init() {
registry.Commands = append(registry.Commands, registry.CliCommand{
Command: locksCommand,
Parent: systemCmd,
})
}
func runLocks() error {
report, err := registry.ContainerEngine().Locks(registry.Context())
if err != nil {
return err
}

for lockNum, objects := range report.LockConflicts {
fmt.Printf("Lock %d is in use by the following\n:", lockNum)
for _, obj := range objects {
fmt.Printf("\t%s\n", obj)
}
}

if len(report.LockConflicts) > 0 {
fmt.Printf("\nLock conflicts have been detected. Recommend immediate use of `podman system renumber` to resolve.\n\n")
} else {
fmt.Printf("\nNo lock conflicts have been detected.\n\n")
}

for _, lockNum := range report.LocksHeld {
fmt.Printf("Lock %d is presently being held\n", lockNum)
}

return nil
}
1 change: 1 addition & 0 deletions docs/source/markdown/podman-container-inspect.1.md.in
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ Valid placeholders for the Go template are listed below:
| .IsInfra | Is this an infra container? (string: true/false) |
| .IsService | Is this a service container? (string: true/false) |
| .KubeExitCodePropagation | Kube exit-code propagation (string) |
| .LockNumber | Number of the container's Libpod lock |
| .MountLabel | SELinux label of mount (string) |
| .Mounts | Mounts (array of strings) |
| .Name | Container name (string) |
Expand Down
1 change: 1 addition & 0 deletions docs/source/markdown/podman-pod-inspect.1.md.in
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ Valid placeholders for the Go template are listed below:
| .InfraContainerID | Pod infrastructure ID |
| .InspectPodData ... | Nested structure, for experts only |
| .Labels | Pod labels |
| .LockNumber | Number of the pod's Libpod lock |
| .MemoryLimit | Memory limit, bytes |
| .MemorySwap | Memory swap limit, in bytes |
| .Mounts | Mounts |
Expand Down
1 change: 1 addition & 0 deletions docs/source/markdown/podman-volume-inspect.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ Valid placeholders for the Go template are listed below:
| .Driver | Volume driver |
| .GID | GID the volume was created with |
| .Labels | Label information associated with the volume |
| .LockNumber | Number of the volume's Libpod lock |
| .MountCount | Number of times the volume is mounted |
| .Mountpoint | Source of volume mount point |
| .Name | Volume name |
Expand Down
1 change: 1 addition & 0 deletions docs/source/markdown/podman-volume-ls.1.md.in
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ Valid placeholders for the Go template are listed below:
| .GID | GID of volume |
| .InspectVolumeData ... | Don't use |
| .Labels | Label information associated with the volume |
| .LockNumber | Number of the volume's Libpod lock |
| .MountCount | Number of times the volume is mounted |
| .Mountpoint | Source of volume mount point |
| .Name | Volume name |
Expand Down
1 change: 1 addition & 0 deletions libpod/container_inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ func (c *Container) getContainerInspectData(size bool, driverData *define.Driver
IsInfra: c.IsInfra(),
IsService: c.IsService(),
KubeExitCodePropagation: config.KubeExitCodePropagation.String(),
LockNumber: c.lock.ID(),
}

if config.RootfsImageID != "" { // May not be set if the container was created with --rootfs
Expand Down
1 change: 1 addition & 0 deletions libpod/define/container_inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,7 @@ type InspectContainerData struct {
IsInfra bool `json:"IsInfra"`
IsService bool `json:"IsService"`
KubeExitCodePropagation string `json:"KubeExitCodePropagation"`
LockNumber uint32 `json:"lockNumber"`
Config *InspectContainerConfig `json:"Config"`
HostConfig *InspectContainerHostConfig `json:"HostConfig"`
}
Expand Down
1 change: 1 addition & 0 deletions libpod/define/info.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ type HostInfo struct {
DatabaseBackend string `json:"databaseBackend"`
Distribution DistributionInfo `json:"distribution"`
EventLogger string `json:"eventLogger"`
FreeLocks *uint32 `json:"freeLocks,omitempty"`
Hostname string `json:"hostname"`
IDMappings IDMappings `json:"idMappings,omitempty"`
Kernel string `json:"kernel"`
Expand Down
2 changes: 2 additions & 0 deletions libpod/define/pod_inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ type InspectPodData struct {
BlkioWeightDevice []InspectBlkioWeightDevice `json:"blkio_weight_device,omitempty"`
// RestartPolicy of the pod.
RestartPolicy string `json:"RestartPolicy,omitempty"`
// Number of the pod's Libpod lock.
LockNumber uint32
}

// InspectPodInfraConfig contains the configuration of the pod's infra
Expand Down
2 changes: 2 additions & 0 deletions libpod/define/volume_inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ type InspectVolumeData struct {
// StorageID is the ID of the container backing the volume in c/storage.
// Only used with Image Volumes.
StorageID string `json:"StorageID,omitempty"`
// LockNumber is the number of the volume's Libpod lock.
LockNumber uint32
}

type VolumeReload struct {
Expand Down
7 changes: 7 additions & 0 deletions libpod/info.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,12 @@ func (r *Runtime) hostInfo() (*define.HostInfo, error) {
if err != nil {
return nil, err
}

locksFree, err := r.lockManager.AvailableLocks()
if err != nil {
return nil, fmt.Errorf("getting free locks: %w", err)
}

info := define.HostInfo{
Arch: runtime.GOARCH,
BuildahVersion: buildah.Version,
Expand All @@ -107,6 +113,7 @@ func (r *Runtime) hostInfo() (*define.HostInfo, error) {
CPUs: runtime.NumCPU(),
CPUUtilization: cpuUtil,
Distribution: hostDistributionInfo,
FreeLocks: locksFree,
LogDriver: r.config.Containers.LogDriver,
EventLogger: r.eventer.String(),
Hostname: host,
Expand Down
16 changes: 16 additions & 0 deletions libpod/lock/file_lock_manager.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package lock

import (
"github.com/containers/podman/v4/libpod/define"
"github.com/containers/podman/v4/libpod/lock/file"
)

Expand Down Expand Up @@ -79,6 +80,21 @@ func (m *FileLockManager) FreeAllLocks() error {
return m.locks.DeallocateAllLocks()
}

// AvailableLocks returns the number of available locks. Since this is not
// limited in the file lock implementation, nil is returned.
func (m *FileLockManager) AvailableLocks() (*uint32, error) {
return nil, nil
}

// LocksHeld returns any locks that are presently locked.
// It is not implemented for the file lock backend.
// It ought to be possible, but my motivation to dig into c/storage and add
// trylock semantics to the filelocker implementation for an uncommonly-used
// lock backend is lacking.
func (m *FileLockManager) LocksHeld() ([]uint32, error) {
return nil, define.ErrNotImplemented
}

// FileLock is an individual shared memory lock.
type FileLock struct {
lockID uint32
Expand Down
30 changes: 30 additions & 0 deletions libpod/lock/in_memory_locks.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,3 +116,33 @@ func (m *InMemoryManager) FreeAllLocks() error {

return nil
}

// Get number of available locks
func (m *InMemoryManager) AvailableLocks() (*uint32, error) {
var count uint32

for _, lock := range m.locks {
if !lock.allocated {
count++
}
}

return &count, nil
}

// Get any locks that are presently being held.
// Useful for debugging deadlocks.
func (m *InMemoryManager) LocksHeld() ([]uint32, error) {
//nolint:prealloc
var locks []uint32

for _, lock := range m.locks {
if lock.lock.TryLock() {
lock.lock.Unlock()
continue
}
locks = append(locks, lock.ID())
}

return locks, nil
}
10 changes: 10 additions & 0 deletions libpod/lock/lock.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,16 @@ type Manager interface {
// renumbering, where reasonable guarantees about other processes can be
// made.
FreeAllLocks() error
// NumAvailableLocks gets the number of remaining locks available to be
// allocated.
// Some lock managers do not have a maximum number of locks, and can
// allocate an unlimited number. These implementations should return
// a nil uin32.
AvailableLocks() (*uint32, error)
// Get a list of locks that are currently locked.
// This may not be supported by some drivers, depending on the exact
// backend implementation in use.
LocksHeld() ([]uint32, error)
}

// Locker is similar to sync.Locker, but provides a method for freeing the lock
Expand Down
119 changes: 110 additions & 9 deletions libpod/lock/shm/shm_lock.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,18 @@ static size_t compute_shm_size(uint32_t num_bitmaps) {
// Handles exceptional conditions, including a mutex locked by a process that
// died holding it.
// Returns 0 on success, or positive errno on failure.
static int take_mutex(pthread_mutex_t *mutex) {
static int take_mutex(pthread_mutex_t *mutex, bool trylock) {
int ret_code;

do {
ret_code = pthread_mutex_lock(mutex);
} while(ret_code == EAGAIN);
if (!trylock) {
do {
ret_code = pthread_mutex_lock(mutex);
} while(ret_code == EAGAIN);
} else {
do {
ret_code = pthread_mutex_trylock(mutex);
} while(ret_code == EAGAIN);
}

if (ret_code == EOWNERDEAD) {
// The previous owner of the mutex died while holding it
Expand Down Expand Up @@ -309,7 +315,7 @@ int64_t allocate_semaphore(shm_struct_t *shm) {
}

// Lock the semaphore controlling access to our shared memory
ret_code = take_mutex(&(shm->segment_lock));
ret_code = take_mutex(&(shm->segment_lock), false);
if (ret_code != 0) {
return -1 * ret_code;
}
Expand Down Expand Up @@ -383,7 +389,7 @@ int32_t allocate_given_semaphore(shm_struct_t *shm, uint32_t sem_index) {
test_map = 0x1 << index_in_bitmap;

// Lock the mutex controlling access to our shared memory
ret_code = take_mutex(&(shm->segment_lock));
ret_code = take_mutex(&(shm->segment_lock), false);
if (ret_code != 0) {
return -1 * ret_code;
}
Expand Down Expand Up @@ -436,7 +442,7 @@ int32_t deallocate_semaphore(shm_struct_t *shm, uint32_t sem_index) {
test_map = 0x1 << index_in_bitmap;

// Lock the mutex controlling access to our shared memory
ret_code = take_mutex(&(shm->segment_lock));
ret_code = take_mutex(&(shm->segment_lock), false);
if (ret_code != 0) {
return -1 * ret_code;
}
Expand Down Expand Up @@ -475,7 +481,7 @@ int32_t deallocate_all_semaphores(shm_struct_t *shm) {
}

// Lock the mutex controlling access to our shared memory
ret_code = take_mutex(&(shm->segment_lock));
ret_code = take_mutex(&(shm->segment_lock), false);
if (ret_code != 0) {
return -1 * ret_code;
}
Expand Down Expand Up @@ -513,7 +519,7 @@ int32_t lock_semaphore(shm_struct_t *shm, uint32_t sem_index) {
bitmap_index = sem_index / BITMAP_SIZE;
index_in_bitmap = sem_index % BITMAP_SIZE;

return -1 * take_mutex(&(shm->locks[bitmap_index].locks[index_in_bitmap]));
return -1 * take_mutex(&(shm->locks[bitmap_index].locks[index_in_bitmap]), false);
}

// Unlock a given semaphore
Expand All @@ -537,3 +543,98 @@ int32_t unlock_semaphore(shm_struct_t *shm, uint32_t sem_index) {

return -1 * release_mutex(&(shm->locks[bitmap_index].locks[index_in_bitmap]));
}

// Get the number of free locks.
// Returns a positive integer guaranteed to be less than UINT32_MAX on success,
// or negative errno values on failure.
// On success, the returned integer is the number of free semaphores.
int64_t available_locks(shm_struct_t *shm) {
int ret_code, i, count;
bitmap_t test_map;
int64_t free_locks = 0;

if (shm == NULL) {
return -1 * EINVAL;
}

// Lock the semaphore controlling access to the SHM segment.
// This isn't strictly necessary as we're only reading, but it seems safer.
ret_code = take_mutex(&(shm->segment_lock), false);
if (ret_code != 0) {
return -1 * ret_code;
}

// Loop through all bitmaps, counting number of allocated locks.
for (i = 0; i < shm->num_bitmaps; i++) {
// Short-circuit to catch fully-empty bitmaps quick.
if (shm->locks[i].bitmap == 0) {
free_locks += sizeof(bitmap_t) * 8;
continue;
}

// Use Kernighan's Algorithm to count bits set. Subtract from number of bits
// in the integer to get free bits, and thus free lock count.
test_map = shm->locks[i].bitmap;
count = 0;
while (test_map) {
test_map = test_map & (test_map - 1);
count++;
}

free_locks += (sizeof(bitmap_t) * 8) - count;
}

// Clear the mutex
ret_code = release_mutex(&(shm->segment_lock));
if (ret_code != 0) {
return -1 * ret_code;
}

// Return free lock count.
return free_locks;
}

// Attempt to take a given semaphore. If successfully taken, it is immediately
// released before the function returns.
// Used to check if a semaphore is in use, to detect potential deadlocks where a
// lock has not been released for an extended period of time.
// Note that this is NOT POSIX trylock as the lock is immediately released if
// taken.
// Returns negative errno on failure.
// On success, returns 1 if the lock was successfully taken, and 0 if it was
// not.
int32_t try_lock(shm_struct_t *shm, uint32_t sem_index) {
int bitmap_index, index_in_bitmap, ret_code;
pthread_mutex_t *mutex;

if (shm == NULL) {
return -1 * EINVAL;
}

if (sem_index >= shm->num_locks) {
return -1 * EINVAL;
}

bitmap_index = sem_index / BITMAP_SIZE;
index_in_bitmap = sem_index % BITMAP_SIZE;

mutex = &(shm->locks[bitmap_index].locks[index_in_bitmap]);

ret_code = take_mutex(mutex, true);

if (ret_code == EBUSY) {
// Did not successfully take the lock
return 0;
} else if (ret_code != 0) {
// Another, unrelated error
return -1 * ret_code;
}

// Lock taken successfully, unlock and return.
ret_code = release_mutex(mutex);
if (ret_code != 0) {
return -1 * ret_code;
}

return 1;
}
Loading