From cef80a1fb7edf58f8050d2ac32c8c36e000c2163 Mon Sep 17 00:00:00 2001 From: Adrien Destugues Date: Sun, 17 Jan 2021 17:46:05 +0100 Subject: [PATCH] devfs: translate partition offsets in B_TRIM_DEVICE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes bfs part of #10336. Untested on SATA (don't have a testing drive to sacrifice) but working fine on SD/MMC. This requires moving the copy from kernel to userland into the devfs. As a result the code in the disk drivers becomes a bit simpler. Also add some documentation for the common ioctls to implement for a disk device. Change-Id: Ie84b6a1d293828d33902a64b3c9d4b19aa6eacb1 Reviewed-on: https://review.haiku-os.org/c/haiku/+/3640 Reviewed-by: Jérôme Duval --- docs/develop/drivers/disk/ioctls.txt | 42 +++++++++++++++++ headers/os/drivers/Drivers.h | 4 +- .../drivers/disk/scsi/scsi_disk/scsi_disk.cpp | 16 ++----- .../disk/virtual/ram_disk/ram_disk.cpp | 16 ++----- src/system/kernel/device_manager/devfs.cpp | 46 +++++++++++++++++-- 5 files changed, 93 insertions(+), 31 deletions(-) create mode 100644 docs/develop/drivers/disk/ioctls.txt diff --git a/docs/develop/drivers/disk/ioctls.txt b/docs/develop/drivers/disk/ioctls.txt new file mode 100644 index 0000000000..1f6a3e3adc --- /dev/null +++ b/docs/develop/drivers/disk/ioctls.txt @@ -0,0 +1,42 @@ +Here is a list of ioctls usually implemented by disk devices. + +B_GET_DEVICE_SIZE + +The parameter is a size_t and is filled with the disk size in bytes. +This is limited to 4GB and not very useful. B_GET_GEOMETRY is used instead. + +B_GET_GEOMETRY + +The parameter is a device_geometry structure to be filled with the device geometry. + +B_GET_ICON_NAME + +Deprecated. Get the name of an icon to use. The icons are hardcoded in Tracker. + +B_GET_VECTOR_ICON + +The parameter is a device_icon structure to be populated with the icon data in HVIF format. +This icon is then used to show the disk in Tracker, for example. + +B_EJECT_DEVICE + +Eject the device (for removable devices). + +B_LOAD_MEDIA + +Load the device (reverse of eject) if possible. + +B_FLUSH_DRIVE_CACHE + +Make sure all data is stored on persistent storage and not in caches (including any caching inside +the device) + +B_TRIM_DEVICE + +The parameter is an fs_trim_data structure. It is guaranteed to be in kernel memory because +the partition manager pre-processes requests coming from userland and makes sure no sectors +are outside the partition range for a specific partition device. + +Mark the listed areas on disk as unused, allowing future reads to these areas to return +random data or read errors. Flash memory devices (SSD, MMC, ...) may use this information +to optimize their internal storage. diff --git a/headers/os/drivers/Drivers.h b/headers/os/drivers/Drivers.h index a854305b38..a8b4d7f788 100644 --- a/headers/os/drivers/Drivers.h +++ b/headers/os/drivers/Drivers.h @@ -175,8 +175,8 @@ typedef struct { uint32 range_count; uint64 trimmed_size; /* filled on return */ struct range { - uint64 offset; /* offset (in bytes) */ - uint64 size; + off_t offset; /* offset (in bytes) */ + off_t size; } ranges[1]; } fs_trim_data; diff --git a/src/add-ons/kernel/drivers/disk/scsi/scsi_disk/scsi_disk.cpp b/src/add-ons/kernel/drivers/disk/scsi/scsi_disk/scsi_disk.cpp index 05246eb063..0d6b1188df 100644 --- a/src/add-ons/kernel/drivers/disk/scsi/scsi_disk/scsi_disk.cpp +++ b/src/add-ons/kernel/drivers/disk/scsi/scsi_disk/scsi_disk.cpp @@ -418,18 +418,10 @@ das_ioctl(void* cookie, uint32 op, void* buffer, size_t length) #if 0 case B_TRIM_DEVICE: { - fs_trim_data* trimData; - MemoryDeleter deleter; - status_t status = get_trim_data_from_user(buffer, length, deleter, - trimData); - if (status != B_OK) - return status; - - status = trim_device(info, trimData); - if (status != B_OK) - return status; - - return copy_trim_data_to_user(buffer, trimData); + // We know the buffer is kernel-side because it has been + // preprocessed in devfs + ASSERT(IS_KERNEL_ADDRESS(buffer)); + return trim_device(info, (fs_trim_data*)buffer); } #endif diff --git a/src/add-ons/kernel/drivers/disk/virtual/ram_disk/ram_disk.cpp b/src/add-ons/kernel/drivers/disk/virtual/ram_disk/ram_disk.cpp index fdb3200be9..280e370abb 100644 --- a/src/add-ons/kernel/drivers/disk/virtual/ram_disk/ram_disk.cpp +++ b/src/add-ons/kernel/drivers/disk/virtual/ram_disk/ram_disk.cpp @@ -1474,18 +1474,10 @@ ram_disk_raw_device_control(void* _cookie, uint32 op, void* buffer, case B_TRIM_DEVICE: { - fs_trim_data* trimData; - MemoryDeleter deleter; - status_t status = get_trim_data_from_user(buffer, length, deleter, - trimData); - if (status != B_OK) - return status; - - status = device->Trim(trimData); - if (status != B_OK) - return status; - - return copy_trim_data_to_user(buffer, trimData); + // We know the buffer is kernel-side because it has been + // preprocessed in devfs + ASSERT(IS_KERNEL_ADDRESS(buffer)); + return device->Trim((fs_trim_data*)buffer); } case RAM_DISK_IOCTL_INFO: diff --git a/src/system/kernel/device_manager/devfs.cpp b/src/system/kernel/device_manager/devfs.cpp index 65b9ce7cc6..17a0730c06 100644 --- a/src/system/kernel/device_manager/devfs.cpp +++ b/src/system/kernel/device_manager/devfs.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -493,14 +494,14 @@ err1: } -static inline void +template static inline void translate_partition_access(devfs_partition* partition, off_t& offset, - size_t& size) + size_type& size) { ASSERT(offset >= 0); ASSERT(offset < partition->info.size); - size = (size_t)min_c((off_t)size, partition->info.size - offset); + size = (size_type)min_c((off_t)size, partition->info.size - offset); offset += partition->info.offset; } @@ -1214,7 +1215,8 @@ devfs_read(fs_volume* _volume, fs_vnode* _vnode, void* _cookie, off_t pos, if (pos >= vnode->stream.u.dev.partition->info.size) return B_BAD_VALUE; - translate_partition_access(vnode->stream.u.dev.partition, pos, *_length); + translate_partition_access(vnode->stream.u.dev.partition, pos, + *_length); } if (*_length == 0) @@ -1246,7 +1248,8 @@ devfs_write(fs_volume* _volume, fs_vnode* _vnode, void* _cookie, off_t pos, if (pos >= vnode->stream.u.dev.partition->info.size) return B_BAD_VALUE; - translate_partition_access(vnode->stream.u.dev.partition, pos, *_length); + translate_partition_access(vnode->stream.u.dev.partition, pos, + *_length); } if (*_length == 0) @@ -1460,6 +1463,39 @@ devfs_ioctl(fs_volume* _volume, fs_vnode* _vnode, void* _cookie, uint32 op, return user_memcpy(buffer, &geometry, sizeof(device_geometry)); } + case B_TRIM_DEVICE: + { + struct devfs_partition* partition + = vnode->stream.u.dev.partition; + + fs_trim_data* trimData; + MemoryDeleter deleter; + status_t status = get_trim_data_from_user(buffer, length, + deleter, trimData); + if (status != B_OK) + return status; + + if (partition != NULL) { + // If there is a partition, offset all ranges according + // to the partition start. + for (uint32 i = 0; i < trimData->range_count; i++) { + translate_partition_access(partition, + trimData->ranges[i].offset, + trimData->ranges[i].size); + } + } + + status = vnode->stream.u.dev.device->Control( + cookie->device_cookie, op, trimData, length); + + // Copy the data back to userland (it contains the number of + // trimmed bytes) + if (status == B_OK) + status = copy_trim_data_to_user(buffer, trimData); + + return status; + } + case B_GET_DRIVER_FOR_DEVICE: { #if 0