From b8a803ccbb6e403365be28ee80c9d7a6857fd327 Mon Sep 17 00:00:00 2001 From: Sven Anderson Date: Sat, 16 Jan 2021 21:14:58 +0100 Subject: [PATCH] cutil: use SyncBuffer for iovec type The main motivation for PtrGuard was read and write buffers as they are used in iovec. This change uses SyncBuffer for the iovec implementation, so that the no-copy PtrGuard implementation can be enabled with the with_ptrguard build tag. Signed-off-by: Sven Anderson --- cephfs/file.go | 1 + internal/cutil/iovec.go | 80 ++++++++++++------------------- internal/cutil/iovec_test.go | 91 ++++++++++++++++++------------------ 3 files changed, 78 insertions(+), 94 deletions(-) diff --git a/cephfs/file.go b/cephfs/file.go index 83c49b8..2b5c74e 100644 --- a/cephfs/file.go +++ b/cephfs/file.go @@ -158,6 +158,7 @@ func (f *File) Preadv(data [][]byte, offset int64) (int, error) { case ret == 0: return 0, io.EOF } + iov.Sync() return int(ret), nil } diff --git a/internal/cutil/iovec.go b/internal/cutil/iovec.go index d89de06..a711369 100644 --- a/internal/cutil/iovec.go +++ b/internal/cutil/iovec.go @@ -5,74 +5,56 @@ package cutil #include */ import "C" - import ( "unsafe" ) -var iovecSize uintptr - -// StructIovecPtr is an unsafe pointer wrapping C's `*struct iovec`. -type StructIovecPtr unsafe.Pointer - -// Iovec helps manage struct iovec arrays needed by some C functions. +// Iovec is a slice of iovec structs. Might have allocated C memory, so it must +// be freed with the Free() method. type Iovec struct { - // cvec represents an array of struct iovec C memory - cvec unsafe.Pointer - // length of the array (in elements) - length int + iovec []C.struct_iovec + sbs []*SyncBuffer } -// NewIovec creates an Iovec, and underlying C memory, of the specified size. -func NewIovec(l int) *Iovec { - r := &Iovec{ - cvec: C.malloc(C.size_t(l) * C.size_t(iovecSize)), - length: l, +const iovecSize = C.sizeof_struct_iovec + +// ByteSlicesToIovec creates an Iovec and links it to Go buffers in data. +func ByteSlicesToIovec(data [][]byte) (v Iovec) { + n := len(data) + iovecMem := C.malloc(iovecSize * C.size_t(n)) + v.iovec = (*[MaxIdx]C.struct_iovec)(iovecMem)[:n:n] + for i, b := range data { + sb := NewSyncBuffer(CPtr(&v.iovec[i].iov_base), b) + v.sbs = append(v.sbs, sb) + v.iovec[i].iov_len = C.size_t(len(b)) } - return r + return } -// ByteSlicesToIovec takes a slice of byte slices and returns a new iovec that -// maps the slice data to struct iovec entries. -func ByteSlicesToIovec(data [][]byte) *Iovec { - iov := NewIovec(len(data)) - for i := range data { - iov.Set(i, data[i]) +// Sync makes sure the slices contain the same as the C buffers +func (v *Iovec) Sync() { + for _, sb := range v.sbs { + sb.Sync() } - return iov } -// Pointer returns a StructIovecPtr that represents the C memory of the -// underlying array. -func (v *Iovec) Pointer() StructIovecPtr { - return StructIovecPtr(unsafe.Pointer(v.cvec)) +// Pointer returns a pointer to the iovec +func (v *Iovec) Pointer() unsafe.Pointer { + return unsafe.Pointer(&v.iovec[0]) } -// Len returns the number of entries in the Iovec. +// Len returns a pointer to the iovec func (v *Iovec) Len() int { - return v.length + return len(v.iovec) } // Free the C memory in the Iovec. func (v *Iovec) Free() { - if v.cvec != nil { - C.free(v.cvec) - v.cvec = nil - v.length = 0 + for _, sb := range v.sbs { + sb.Release() } -} - -// Set will map the memory of the given byte slice to the iovec at the -// specified position. -func (v *Iovec) Set(i int, buf []byte) { - offset := uintptr(i) * iovecSize - iov := (*C.struct_iovec)(unsafe.Pointer( - uintptr(unsafe.Pointer(v.cvec)) + offset)) - iov.iov_base = unsafe.Pointer(&buf[0]) - iov.iov_len = C.size_t(len(buf)) -} - -func init() { - var iovec C.struct_iovec - iovecSize = unsafe.Sizeof(iovec) + if len(v.iovec) != 0 { + C.free(unsafe.Pointer(&v.iovec[0])) + } + v.iovec = nil } diff --git a/internal/cutil/iovec_test.go b/internal/cutil/iovec_test.go index 9978877..0534131 100644 --- a/internal/cutil/iovec_test.go +++ b/internal/cutil/iovec_test.go @@ -5,53 +5,54 @@ import ( "unsafe" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestIovec(t *testing.T) { - t.Run("newAndFree", func(t *testing.T) { - iov := NewIovec(3) - iov.Free() - }) - t.Run("setBufs", func(t *testing.T) { - b1 := []byte("foo") - b2 := []byte("barbar") - b3 := []byte("bazbazbaz") - iov := NewIovec(3) - iov.Set(0, b1) - iov.Set(1, b2) - iov.Set(2, b3) - iov.Free() - // free also unsets internal values - assert.Equal(t, unsafe.Pointer(nil), iov.cvec) - assert.Equal(t, 0, iov.length) - }) - t.Run("testGetters", func(t *testing.T) { - b1 := []byte("foo") - b2 := []byte("barbar") - b3 := []byte("bazbazbaz") - b4 := []byte("zonk") - iov := NewIovec(4) - defer iov.Free() - iov.Set(0, b1) - iov.Set(1, b2) - iov.Set(2, b3) - iov.Set(3, b4) - - assert.NotNil(t, iov.Pointer()) - assert.Equal(t, 4, iov.Len()) - }) -} - -func TestByteSlicesToIovec(t *testing.T) { - d := [][]byte{ - []byte("ramekin"), - []byte("shuffleboard"), - []byte("tranche"), - []byte("phycobilisomes"), + strs := []string{ + "foo", + "barbar", + "bazbazbaz", } - iov := ByteSlicesToIovec(d) - defer iov.Free() - - assert.NotNil(t, iov.Pointer()) - assert.Equal(t, 4, iov.Len()) + var data [][]byte + for _, s := range strs { + data = append(data, []byte(s)) + } + iovec := ByteSlicesToIovec(data) + p := iovec.Pointer() + assert.NotNil(t, p) + assert.Equal(t, iovec.Len(), len(data)) + assert.Equal(t, p, unsafe.Pointer(&iovec.iovec[0])) + for i, iov := range iovec.iovec { + require.NotNil(t, iov.iov_base) + assert.Equal(t, int(iov.iov_len), len(data[i])) + assert.Equal(t, data[i], (*[MaxIdx]byte)(iov.iov_base)[:iov.iov_len:iov.iov_len]) + } + // data didn't change + for i, b := range data { + assert.Equal(t, string(b), strs[i]) + } + // clear iovec buffers + for _, iov := range iovec.iovec { + b := (*[MaxIdx]byte)(iov.iov_base)[:iov.iov_len:iov.iov_len] + for i := range b { + b[i] = 0 + } + } + iovec.Sync() + // data must be cleared + for _, b := range data { + for i := range b { + assert.Zero(t, b[i]) + } + } + iovec.Free() + for _, iov := range iovec.iovec { + assert.Equal(t, iov.iov_base, unsafe.Pointer(nil)) + assert.Zero(t, iov.iov_len) + } + iovec.Free() + iovec.Sync() + iovec.Sync() + iovec.Free() }