[COMMIT osv master] nvme: free aligned_new() with free()

[Commit Bot]

From: Nadav Har'El <n...@scylladb.com>
Committer: Nadav Har'El <n...@scylladb.com>
Branch: master

nvme: free aligned_new() with free()

This is the same problem of over-zealos gcc warnings that we already fixed
in commit 3a7c0db761cc65e908b1d3e8ef41d433dad517a8, but this patch introduces
a more general fix that can be used every time we have this problem.

Memory allocated with aligned_new() should be freed with free(), not
with operator delete - although in our actual implemention those are really
the same thing. gcc 14.1.1 started to detect cases where we use std::unique_ptr
with a pointer allocated with aligned_new(), but unique_ptr's default deleter
uses delete.

So in this patch we introduce in aligned_new.hh a new deleter class,
aligned_new_deleter<T>, which can be used with unique_ptr to ensure the
pointer is eventually deleted with free instead of operator delete, and
pacifies the gcc error checker.

In the patch we use this new facility in a couple of places in
drivers/nvme.cc which had this problem.

Signed-off-by: Nadav Har'El <n...@scylladb.com>

---
diff --git a/drivers/nvme.cc b/drivers/nvme.cc
--- a/drivers/nvme.cc
+++ b/drivers/nvme.cc
@@ -306,7 +306,7 @@ void driver::create_admin_queue()
u32* cq_doorbell = (u32*) ((u64)sq_doorbell + _doorbell_stride);

int qsize = NVME_ADMIN_QUEUE_SIZE;
- _admin_queue = std::unique_ptr<admin_queue_pair>(
+ _admin_queue = std::unique_ptr<admin_queue_pair, aligned_new_deleter<admin_queue_pair>>(
aligned_new<admin_queue_pair>(_id, 0, qsize, _dev, sq_doorbell, cq_doorbell, _ns_data));

register_admin_interrupt();
@@ -341,7 +341,7 @@ int driver::create_io_queue(int qid, int qsize, sched::cpu* cpu, int qprio)
u32* cq_doorbell = (u32*) ((u64) sq_doorbell + _doorbell_stride);

// create queue pair with allocated SQ and CQ ring buffers
- auto queue = std::unique_ptr<io_queue_pair>(
+ auto queue = std::unique_ptr<io_queue_pair, aligned_new_deleter<io_queue_pair>>(
aligned_new<io_queue_pair>(_id, iv, qsize, _dev, sq_doorbell, cq_doorbell, _ns_data));

// create completion queue command
diff --git a/drivers/nvme.hh b/drivers/nvme.hh
--- a/drivers/nvme.hh
+++ b/drivers/nvme.hh
@@ -15,6 +15,7 @@
#include <osv/mempool.hh>
#include <osv/interrupt.hh>
#include <osv/msi.hh>
+#include <osv/aligned_new.hh>
#include "drivers/nvme-queue.hh"
#include <vector>
#include <memory>
@@ -96,9 +97,9 @@ private:

std::vector<std::unique_ptr<msix_vector>> _msix_vectors;

- std::unique_ptr<admin_queue_pair> _admin_queue;
+ std::unique_ptr<admin_queue_pair, aligned_new_deleter<admin_queue_pair>> _admin_queue;

- std::vector<std::unique_ptr<io_queue_pair>> _io_queues;
+ std::vector<std::unique_ptr<io_queue_pair, aligned_new_deleter<io_queue_pair>>> _io_queues;
u32 _doorbell_stride;

std::unique_ptr<nvme_identify_ctlr_t> _identify_controller;
diff --git a/include/osv/aligned_new.hh b/include/osv/aligned_new.hh
--- a/include/osv/aligned_new.hh
+++ b/include/osv/aligned_new.hh
@@ -32,6 +32,21 @@ T* aligned_new(Args&&... args) {
return new(p) T(std::forward<Args>(args)...);
}

+// Memory allocated in aligned_new with aligned_alloc() should be freed with
+// free(), not with delete. Although in our actual implemention those are
+// really the same thing, gcc is sometimes able to warn about std::unique_ptr<>
+// being given a pointer created by aligned_new() but the default deleter is
+// "delete". In this case, use the following alternative deleter.
+// For example:
+// auto x = std::unique_ptr<T, aligned_new_deleter<T>(aligned_new<T>(...))
+template<typename T>
+struct aligned_new_deleter {
+ void operator()(T* p) const {
+ p->~T();
+ free(p);
+ }
+};
+
// Similar function for allocating an array of objects. But here we have
// a problem: While an object created with aligned_new<>() can be deleted by
// an ordinary "delete" (as explained above), here, an array allocated by an