Kernel: Simplify ProcFS generated buffer caching

Use a Mutex instead of a SpinLock to protect the per-FileDescription
generated data cache. This allows processes to go to sleep while
waiting their turn.

Also don't try to be clever by reusing existing cache buffers.
Just allocate KBuffers as needed (and make sure to surface failures.)
This commit is contained in:
Andreas Kling 2021-07-20 14:43:15 +02:00
parent 4d2473b7fa
commit a3063dfd33
Notes: sideshowbarker 2024-07-18 08:40:49 +09:00
2 changed files with 28 additions and 37 deletions

View file

@ -69,7 +69,7 @@ ProcFSExposedLink::ProcFSExposedLink(StringView name, InodeIndex preallocated_in
} }
struct ProcFSInodeData : public FileDescriptionData { struct ProcFSInodeData : public FileDescriptionData {
RefPtr<KBufferImpl> buffer; OwnPtr<KBuffer> buffer;
}; };
KResultOr<size_t> ProcFSGlobalInformation::read_bytes(off_t offset, size_t count, UserOrKernelBuffer& buffer, FileDescription* description) const KResultOr<size_t> ProcFSGlobalInformation::read_bytes(off_t offset, size_t count, UserOrKernelBuffer& buffer, FileDescription* description) const
@ -81,13 +81,16 @@ KResultOr<size_t> ProcFSGlobalInformation::read_bytes(off_t offset, size_t count
if (!description) if (!description)
return KResult(EIO); return KResult(EIO);
MutexLocker locker(m_refresh_lock);
if (!description->data()) { if (!description->data()) {
dbgln("ProcFSGlobalInformation: Do not have cached data!"); dbgln("ProcFSGlobalInformation: Do not have cached data!");
return KResult(EIO); return KResult(EIO);
} }
// Be sure to keep a reference to data_buffer while we use it! auto& typed_cached_data = static_cast<ProcFSInodeData&>(*description->data());
RefPtr<KBufferImpl> data_buffer = static_cast<ProcFSInodeData&>(*description->data()).buffer; auto& data_buffer = typed_cached_data.buffer;
if (!data_buffer || (size_t)offset >= data_buffer->size()) if (!data_buffer || (size_t)offset >= data_buffer->size())
return 0; return 0;
@ -101,26 +104,19 @@ KResultOr<size_t> ProcFSGlobalInformation::read_bytes(off_t offset, size_t count
KResult ProcFSGlobalInformation::refresh_data(FileDescription& description) const KResult ProcFSGlobalInformation::refresh_data(FileDescription& description) const
{ {
ScopedSpinLock lock(m_refresh_lock); MutexLocker lock(m_refresh_lock);
auto& cached_data = description.data(); auto& cached_data = description.data();
if (!cached_data) if (!cached_data) {
cached_data = adopt_own_if_nonnull(new (nothrow) ProcFSInodeData); cached_data = adopt_own_if_nonnull(new (nothrow) ProcFSInodeData);
VERIFY(description.data()); if (!cached_data)
auto& buffer = static_cast<ProcFSInodeData&>(*cached_data).buffer; return ENOMEM;
if (buffer) {
// If we're reusing the buffer, reset the size to 0 first. This
// ensures we don't accidentally leak previously written data.
buffer->set_size(0);
} }
KBufferBuilder builder(buffer, true); KBufferBuilder builder;
if (!const_cast<ProcFSGlobalInformation&>(*this).output(builder)) if (!const_cast<ProcFSGlobalInformation&>(*this).output(builder))
return ENOENT; return ENOENT;
// We don't use builder.build() here, which would steal our buffer auto& typed_cached_data = static_cast<ProcFSInodeData&>(*cached_data);
// and turn it into an OwnPtr. Instead, just flush to the buffer so typed_cached_data.buffer = builder.build();
// that we can read all the data that was written. if (!typed_cached_data.buffer)
if (!builder.flush())
return ENOMEM;
if (!buffer)
return ENOMEM; return ENOMEM;
return KSuccess; return KSuccess;
} }
@ -139,8 +135,10 @@ KResultOr<size_t> ProcFSProcessInformation::read_bytes(off_t offset, size_t coun
return KResult(EIO); return KResult(EIO);
} }
// Be sure to keep a reference to data_buffer while we use it! MutexLocker locker(m_refresh_lock);
RefPtr<KBufferImpl> data_buffer = static_cast<ProcFSInodeData&>(*description->data()).buffer;
auto& typed_cached_data = static_cast<ProcFSInodeData&>(*description->data());
auto& data_buffer = typed_cached_data.buffer;
if (!data_buffer || (size_t)offset >= data_buffer->size()) if (!data_buffer || (size_t)offset >= data_buffer->size())
return 0; return 0;
@ -171,26 +169,19 @@ KResult ProcFSProcessInformation::refresh_data(FileDescription& description) con
ScopeGuard guard = [&] { ScopeGuard guard = [&] {
process->ptrace_lock().unlock(); process->ptrace_lock().unlock();
}; };
ScopedSpinLock lock(m_refresh_lock); MutexLocker locker(m_refresh_lock);
auto& cached_data = description.data(); auto& cached_data = description.data();
if (!cached_data) if (!cached_data) {
cached_data = adopt_own_if_nonnull(new (nothrow) ProcFSInodeData); cached_data = adopt_own_if_nonnull(new (nothrow) ProcFSInodeData);
VERIFY(description.data()); if (!cached_data)
auto& buffer = static_cast<ProcFSInodeData&>(*cached_data).buffer; return ENOMEM;
if (buffer) {
// If we're reusing the buffer, reset the size to 0 first. This
// ensures we don't accidentally leak previously written data.
buffer->set_size(0);
} }
KBufferBuilder builder(buffer, true); KBufferBuilder builder;
if (!const_cast<ProcFSProcessInformation&>(*this).output(builder)) if (!const_cast<ProcFSProcessInformation&>(*this).output(builder))
return ENOENT; return ENOENT;
// We don't use builder.build() here, which would steal our buffer auto& typed_cached_data = static_cast<ProcFSInodeData&>(*cached_data);
// and turn it into an OwnPtr. Instead, just flush to the buffer so typed_cached_data.buffer = builder.build();
// that we can read all the data that was written. if (!typed_cached_data.buffer)
if (!builder.flush())
return ENOMEM;
if (!buffer)
return ENOMEM; return ENOMEM;
return KSuccess; return KSuccess;
} }

View file

@ -187,7 +187,7 @@ protected:
virtual KResult refresh_data(FileDescription&) const override; virtual KResult refresh_data(FileDescription&) const override;
virtual bool output(KBufferBuilder& builder) = 0; virtual bool output(KBufferBuilder& builder) = 0;
mutable SpinLock<u8> m_refresh_lock; mutable Mutex m_refresh_lock;
}; };
class ProcFSSystemBoolean : public ProcFSGlobalInformation { class ProcFSSystemBoolean : public ProcFSGlobalInformation {
@ -245,7 +245,7 @@ protected:
virtual bool output(KBufferBuilder& builder) = 0; virtual bool output(KBufferBuilder& builder) = 0;
WeakPtr<ProcFSProcessDirectory> m_parent_directory; WeakPtr<ProcFSProcessDirectory> m_parent_directory;
mutable SpinLock<u8> m_refresh_lock; mutable Mutex m_refresh_lock;
}; };
} }