Kernel/FileSystem: Make Inode::{write,read}_bytes methods non-virtual

We make these methods non-virtual because we want to ensure we properly
enforce locking of the m_inode_lock mutex. Also, for write operations,
we want to call prepare_to_write_data before the actual write. The
previous design required us to ensure the callers do that at various
places which lead to hard-to-find bugs. By moving everything to a place
where we call prepare_to_write_data only once, we eliminate a possibilty
of forgeting to call it on some code path in the kernel.
This commit is contained in:
Liav A 2022-08-06 04:22:20 +03:00 committed by Idan Horowitz
parent 4f4717e351
commit c88cc8557f
Notes: sideshowbarker 2024-07-17 07:07:09 +09:00
20 changed files with 87 additions and 72 deletions

View file

@ -811,9 +811,9 @@ ErrorOr<void> Ext2FSInode::compute_block_list_with_exclusive_locking()
return {};
}
ErrorOr<size_t> Ext2FSInode::read_bytes(off_t offset, size_t count, UserOrKernelBuffer& buffer, OpenFileDescription* description) const
ErrorOr<size_t> Ext2FSInode::read_bytes_locked(off_t offset, size_t count, UserOrKernelBuffer& buffer, OpenFileDescription* description) const
{
MutexLocker inode_locker(m_inode_lock);
VERIFY(m_inode_lock.is_locked());
VERIFY(offset >= 0);
if (m_raw_inode.i_size == 0)
return 0;
@ -952,7 +952,7 @@ ErrorOr<void> Ext2FSInode::resize(u64 new_size)
return {};
}
ErrorOr<size_t> Ext2FSInode::write_bytes(off_t offset, size_t count, UserOrKernelBuffer const& data, OpenFileDescription* description)
ErrorOr<size_t> Ext2FSInode::write_bytes_locked(off_t offset, size_t count, UserOrKernelBuffer const& data, OpenFileDescription* description)
{
VERIFY(m_inode_lock.is_locked());
VERIFY(offset >= 0);
@ -963,7 +963,7 @@ ErrorOr<size_t> Ext2FSInode::write_bytes(off_t offset, size_t count, UserOrKerne
if (is_symlink()) {
VERIFY(offset == 0);
if (max((size_t)(offset + count), (size_t)m_raw_inode.i_size) < max_inline_symlink_length) {
dbgln_if(EXT2_DEBUG, "Ext2FSInode[{}]::write_bytes(): Poking into i_block array for inline symlink ({} bytes)", identifier(), count);
dbgln_if(EXT2_DEBUG, "Ext2FSInode[{}]::write_bytes_locked(): Poking into i_block array for inline symlink ({} bytes)", identifier(), count);
TRY(data.read(((u8*)m_raw_inode.i_block) + offset, count));
if ((size_t)(offset + count) > (size_t)m_raw_inode.i_size)
m_raw_inode.i_size = offset + count;
@ -997,14 +997,14 @@ ErrorOr<size_t> Ext2FSInode::write_bytes(off_t offset, size_t count, UserOrKerne
size_t nwritten = 0;
auto remaining_count = min((off_t)count, (off_t)new_size - offset);
dbgln_if(EXT2_VERY_DEBUG, "Ext2FSInode[{}]::write_bytes(): Writing {} bytes, {} bytes into inode from {}", identifier(), count, offset, data.user_or_kernel_ptr());
dbgln_if(EXT2_VERY_DEBUG, "Ext2FSInode[{}]::write_bytes_locked(): Writing {} bytes, {} bytes into inode from {}", identifier(), count, offset, data.user_or_kernel_ptr());
for (auto bi = first_block_logical_index; remaining_count && bi <= last_block_logical_index; bi = bi.value() + 1) {
size_t offset_into_block = (bi == first_block_logical_index) ? offset_into_first_block : 0;
size_t num_bytes_to_copy = min((size_t)block_size - offset_into_block, (size_t)remaining_count);
dbgln_if(EXT2_DEBUG, "Ext2FSInode[{}]::write_bytes(): Writing block {} (offset_into_block: {})", identifier(), m_block_list[bi.value()], offset_into_block);
dbgln_if(EXT2_DEBUG, "Ext2FSInode[{}]::write_bytes_locked(): Writing block {} (offset_into_block: {})", identifier(), m_block_list[bi.value()], offset_into_block);
if (auto result = fs().write_block(m_block_list[bi.value()], data.offset(nwritten), num_bytes_to_copy, offset_into_block, allow_cache); result.is_error()) {
dbgln("Ext2FSInode[{}]::write_bytes(): Failed to write block {} (index {})", identifier(), m_block_list[bi.value()], bi);
dbgln("Ext2FSInode[{}]::write_bytes_locked(): Failed to write block {} (index {})", identifier(), m_block_list[bi.value()], bi);
return result.release_error();
}
remaining_count -= num_bytes_to_copy;
@ -1013,7 +1013,7 @@ ErrorOr<size_t> Ext2FSInode::write_bytes(off_t offset, size_t count, UserOrKerne
did_modify_contents();
dbgln_if(EXT2_VERY_DEBUG, "Ext2FSInode[{}]::write_bytes(): After write, i_size={}, i_blocks={} ({} blocks in list)", identifier(), size(), m_raw_inode.i_blocks, m_block_list.size());
dbgln_if(EXT2_VERY_DEBUG, "Ext2FSInode[{}]::write_bytes_locked(): After write, i_size={}, i_blocks={} ({} blocks in list)", identifier(), size(), m_raw_inode.i_blocks, m_block_list.size());
return nwritten;
}