[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH 3/6] libpager: Add error handling to various functions
|
From: |
Sergey Bugaev |
|
Subject: |
[PATCH 3/6] libpager: Add error handling to various functions |
|
Date: |
Thu, 6 May 2021 15:56:28 +0300 |
Instead of ignoring errors, we might as well return them to the caller.
This technically changes the function signatures, but should not break
any users since they can continue to ignore the return value.
---
doc/hurd.texi | 16 ++++++------
libpager/data-unlock.c | 13 +++++-----
libpager/lock-object.c | 32 +++++++++++++++--------
libpager/offer-page.c | 14 +++++++----
libpager/pager-attr.c | 56 +++++++++++++++++++++++------------------
libpager/pager-flush.c | 21 +++++++++-------
libpager/pager-return.c | 21 +++++++++-------
libpager/pager-sync.c | 19 ++++++++------
libpager/pager.h | 20 +++++++--------
libpager/priv.h | 4 +--
10 files changed, 125 insertions(+), 91 deletions(-)
diff --git a/doc/hurd.texi b/doc/hurd.texi
index f63cfcda..c8db86b5 100644
--- a/doc/hurd.texi
+++ b/doc/hurd.texi
@@ -1434,8 +1434,8 @@ Demultiplex incoming @code{libports} messages on pager
ports.
The following functions are the body of the pager library, and provide a
clean interface to pager functionality:
-@deftypefun void pager_sync (@w{struct pager *@var{pager}}, @w{int @var{wait}})
-@deftypefunx void pager_sync_some (@w{struct pager *@var{pager}},
@w{vm_address_t @var{start}}, @w{vm_size_t @var{len}}, @w{int @var{wait}})
+@deftypefun error_t pager_sync (@w{struct pager *@var{pager}}, @w{int
@var{wait}})
+@deftypefunx error_t pager_sync_some (@w{struct pager *@var{pager}},
@w{vm_address_t @var{start}}, @w{vm_size_t @var{len}}, @w{int @var{wait}})
Write data from pager @var{pager} to its backing store. Wait for all
the writes to complete if and only if @var{wait} is set.
@@ -1443,8 +1443,8 @@ the writes to complete if and only if @var{wait} is set.
data starting at @var{start}, for @var{len} bytes.
@end deftypefun
-@deftypefun void pager_flush (@w{struct pager *@var{pager}}, @w{int
@var{wait}})
-@deftypefunx void pager_flush_some (@w{struct pager *@var{pager}},
@w{vm_address_t @var{start}}, @w{vm_size_t @var{len}}, @w{int @var{wait}})
+@deftypefun error_t pager_flush (@w{struct pager *@var{pager}}, @w{int
@var{wait}})
+@deftypefunx error_t pager_flush_some (@w{struct pager *@var{pager}},
@w{vm_address_t @var{start}}, @w{vm_size_t @var{len}}, @w{int @var{wait}})
Flush data from the kernel for pager @var{pager} and force any pending
delayed copies. Wait for all pages to be flushed if and only if
@var{wait} is set.
@@ -1453,8 +1453,8 @@ delayed copies. Wait for all pages to be flushed if and
only if
flushes data starting at @var{start}, for @var{len} bytes.
@end deftypefun
-@deftypefun void pager_return (@w{struct pager *@var{pager}}, @w{int
@var{wait}})
-@deftypefunx void pager_return_some (@w{struct pager *@var{pager}},
@w{vm_address_t @var{start}}, @w{vm_size_t @var{len}}, @w{int @var{wait}})
+@deftypefun error_t pager_return (@w{struct pager *@var{pager}}, @w{int
@var{wait}})
+@deftypefunx error_t pager_return_some (@w{struct pager *@var{pager}},
@w{vm_address_t @var{start}}, @w{vm_size_t @var{len}}, @w{int @var{wait}})
Flush data from the kernel for pager @var{pager} and force any pending
delayed copies. Wait for all pages to be flushed if and only if
@var{wait} is set. Have the kernel write back modifications.
@@ -1464,14 +1464,14 @@ delayed copies. Wait for all pages to be flushed if
and only if
@var{start}, for @var{len} bytes.
@end deftypefun
-@deftypefun void pager_offer_page (@w{struct pager *@var{pager}}, @w{int
@var{precious}}, @w{int @var{writelock}}, @w{vm_offset_t @var{page}},
@w{vm_address_t @var{buf}})
+@deftypefun error_t pager_offer_page (@w{struct pager *@var{pager}}, @w{int
@var{precious}}, @w{int @var{writelock}}, @w{vm_offset_t @var{page}},
@w{vm_address_t @var{buf}})
Offer a page of data to the kernel. If @var{precious} is set, then this
page will be paged out at some future point, otherwise it might be
dropped by the kernel. If the page is currently in core, the kernel
might ignore this call.
@end deftypefun
-@deftypefun void pager_change_attributes (@w{struct pager *@var{pager}},
@w{boolean_t @var{may_cache}}, @w{memory_object_copy_strategy_t
@var{copy_strategy}}, @w{int @var{wait}})
+@deftypefun error_t pager_change_attributes (@w{struct pager *@var{pager}},
@w{boolean_t @var{may_cache}}, @w{memory_object_copy_strategy_t
@var{copy_strategy}}, @w{int @var{wait}})
Change the attributes of the memory object underlying pager @var{pager}.
The @var{may_cache} and @var{copy_strategy} arguments are as for
@code{memory_object_change_}. Wait for the kernel to report
diff --git a/libpager/data-unlock.c b/libpager/data-unlock.c
index 077e673f..caaea41a 100644
--- a/libpager/data-unlock.c
+++ b/libpager/data-unlock.c
@@ -28,7 +28,7 @@ _pager_S_memory_object_data_unlock (struct pager *p,
vm_size_t length,
vm_prot_t access)
{
- volatile int err;
+ error_t err = 0;
if (!p
|| p->port.class != _pager_class)
@@ -66,16 +66,17 @@ _pager_S_memory_object_data_unlock (struct pager *p,
if (!err)
/* We can go ahead and release the lock. */
- _pager_lock_object (p, offset, length, MEMORY_OBJECT_RETURN_NONE, 0,
- VM_PROT_NONE, 0);
+ err = _pager_lock_object (p, offset, length, MEMORY_OBJECT_RETURN_NONE,
+ 0, VM_PROT_NONE, 0);
else
{
/* Flush the page, and set a bit so that m_o_data_request knows
to issue an error. */
- _pager_lock_object (p, offset, length, MEMORY_OBJECT_RETURN_NONE, 1,
- VM_PROT_WRITE, 0);
_pager_mark_next_request_error (p, offset, length, err);
+ err = _pager_lock_object (p, offset, length, MEMORY_OBJECT_RETURN_NONE,
+ 1, VM_PROT_WRITE, 0);
}
+
out:
- return 0;
+ return err;
}
diff --git a/libpager/lock-object.c b/libpager/lock-object.c
index c022d0c4..96e0cbb0 100644
--- a/libpager/lock-object.c
+++ b/libpager/lock-object.c
@@ -21,8 +21,8 @@
SIZE, SHOULD_RETURN, SHOULD_FLUSH, and LOCK_VALUE are as for
memory_object_lock_request. If SYNC is set, then wait for the
operation to fully complete before returning. */
-void
-_pager_lock_object (struct pager *p,
+error_t
+_pager_lock_object (struct pager *p,
vm_offset_t offset,
vm_size_t size,
int should_return,
@@ -30,14 +30,15 @@ _pager_lock_object (struct pager *p,
vm_prot_t lock_value,
int sync)
{
+ error_t err = 0;
int i;
struct lock_request *lr = 0;
pthread_mutex_lock (&p->interlock);
if (p->pager_state != NORMAL)
{
- pthread_mutex_unlock (&p->interlock);
- return;
+ err = EGRATUITOUS;
+ goto release_out;
}
if (sync)
@@ -52,6 +53,11 @@ _pager_lock_object (struct pager *p,
if (!lr)
{
lr = malloc (sizeof (struct lock_request));
+ if (!lr)
+ {
+ err = errno;
+ goto release_out;
+ }
lr->start = offset;
lr->end = offset + size;
lr->pending_writes = 0;
@@ -66,16 +72,18 @@ _pager_lock_object (struct pager *p,
}
pthread_mutex_unlock (&p->interlock);
- memory_object_lock_request (p->memobjcntl, offset, size, should_return,
- should_flush, lock_value,
- sync ? p->port.port_right : MACH_PORT_NULL);
+ err = memory_object_lock_request (p->memobjcntl, offset, size,
+ should_return, should_flush, lock_value,
+ sync ? p->port.port_right :
MACH_PORT_NULL);
+ if (err)
+ return err;
pthread_mutex_lock (&p->interlock);
-
+
if (sync)
{
while (lr->locks_pending || lr->pending_writes)
pthread_cond_wait (&p->wakeup, &p->interlock);
-
+
if (! --lr->threads_waiting)
{
*lr->prevp = lr->next;
@@ -88,7 +96,9 @@ _pager_lock_object (struct pager *p,
{
vm_offset_t pm_offs = offset / __vm_page_size;
- _pager_pagemap_resize (p, offset + size);
+ err = _pager_pagemap_resize (p, offset + size);
+ if (err)
+ goto release_out;
if (p->pagemapsize > pm_offs)
{
short *pm_entries = &p->pagemap[pm_offs];
@@ -103,5 +113,7 @@ _pager_lock_object (struct pager *p,
}
}
+ release_out:
pthread_mutex_unlock (&p->interlock);
+ return err;
}
diff --git a/libpager/offer-page.c b/libpager/offer-page.c
index 0b0ca2eb..26f88ca3 100644
--- a/libpager/offer-page.c
+++ b/libpager/offer-page.c
@@ -21,25 +21,29 @@
#include "priv.h"
-void
+error_t
pager_offer_page (struct pager *p,
int precious,
int writelock,
vm_offset_t offset,
vm_address_t buf)
{
+ error_t err = 0;
+
pthread_mutex_lock (&p->interlock);
- if (_pager_pagemap_resize (p, offset + vm_page_size))
+ err = _pager_pagemap_resize (p, offset + vm_page_size);
+ if (err)
goto release_out;
short *pm_entry = &p->pagemap[offset / vm_page_size];
*pm_entry |= PM_INCORE;
- memory_object_data_supply (p->memobjcntl, offset, buf, vm_page_size, 0,
- writelock ? VM_PROT_WRITE : VM_PROT_NONE,
- precious, MACH_PORT_NULL);
+ err = memory_object_data_supply (p->memobjcntl, offset, buf, vm_page_size, 0,
+ writelock ? VM_PROT_WRITE : VM_PROT_NONE,
+ precious, MACH_PORT_NULL);
release_out:
pthread_mutex_unlock (&p->interlock);
+ return err;
}
diff --git a/libpager/pager-attr.c b/libpager/pager-attr.c
index 4280e26d..9f55a593 100644
--- a/libpager/pager-attr.c
+++ b/libpager/pager-attr.c
@@ -22,14 +22,15 @@
Arguments MAY_CACHE and COPY_STRATEGY are as for
memory_object_change_attributes. Wait for the kernel to report
completion if WAIT is set. */
-void
+error_t
pager_change_attributes (struct pager *p,
boolean_t may_cache,
memory_object_copy_strategy_t copy_strategy,
int wait)
{
+ error_t err = 0;
struct attribute_request *ar = 0;
-
+
pthread_mutex_lock (&p->interlock);
/* If there's nothing to do we might be able to return. However,
@@ -40,22 +41,22 @@ pager_change_attributes (struct pager *p,
&& ! (p->attribute_requests && wait))
{
pthread_mutex_unlock (&p->interlock);
- return;
+ return 0;
}
p->may_cache = may_cache;
p->copy_strategy = copy_strategy;
-
+
if (p->pager_state == NOTINIT)
{
- pthread_mutex_unlock (&p->interlock);
- return;
+ err = EGRATUITOUS;
+ goto release_out;
}
if (wait)
{
for (ar = p->attribute_requests; ar; ar = ar->next)
- if (ar->may_cache == may_cache
+ if (ar->may_cache == may_cache
&& ar->copy_strategy == copy_strategy)
{
ar->attrs_pending++;
@@ -65,6 +66,11 @@ pager_change_attributes (struct pager *p,
if (!ar)
{
ar = malloc (sizeof (struct attribute_request));
+ if (!ar)
+ {
+ err = errno;
+ goto release_out;
+ }
ar->may_cache = may_cache;
ar->copy_strategy = copy_strategy;
ar->attrs_pending = 1;
@@ -75,27 +81,29 @@ pager_change_attributes (struct pager *p,
ar->prevp = &p->attribute_requests;
p->attribute_requests = ar;
}
- }
+ }
pthread_mutex_unlock (&p->interlock);
- memory_object_change_attributes (p->memobjcntl, may_cache, copy_strategy,
- wait ? p->port.port_right : MACH_PORT_NULL);
-
- if (wait)
- {
- pthread_mutex_lock (&p->interlock);
+ err = memory_object_change_attributes (p->memobjcntl, may_cache,
+ copy_strategy,
+ wait ? p->port.port_right :
MACH_PORT_NULL);
+ if (err || !wait)
+ return err;
- while (ar->attrs_pending)
- pthread_cond_wait (&p->wakeup, &p->interlock);
+ pthread_mutex_lock (&p->interlock);
- if (! --ar->threads_waiting)
- {
- *ar->prevp = ar->next;
- if (ar->next)
- ar->next->prevp = ar->prevp;
- free (ar);
- }
+ while (ar->attrs_pending)
+ pthread_cond_wait (&p->wakeup, &p->interlock);
- pthread_mutex_unlock (&p->interlock);
+ if (! --ar->threads_waiting)
+ {
+ *ar->prevp = ar->next;
+ if (ar->next)
+ ar->next->prevp = ar->prevp;
+ free (ar);
}
+
+ release_out:
+ pthread_mutex_unlock (&p->interlock);
+ return err;
}
diff --git a/libpager/pager-flush.c b/libpager/pager-flush.c
index bd7697ca..cbd05175 100644
--- a/libpager/pager-flush.c
+++ b/libpager/pager-flush.c
@@ -19,26 +19,29 @@
/* Have the kernel flush all pages in the pager; if WAIT is set, then
wait for them to be finally expunged before returning. */
-void
+error_t
pager_flush (struct pager *p, int wait)
{
+ error_t err;
vm_address_t offset;
vm_size_t len;
-
- pager_report_extent (p->upi, &offset, &len);
-
- _pager_lock_object (p, offset, len, MEMORY_OBJECT_RETURN_NONE, 1,
- VM_PROT_NO_CHANGE, wait);
+
+ err = pager_report_extent (p->upi, &offset, &len);
+ if (err)
+ return err;
+
+ return _pager_lock_object (p, offset, len, MEMORY_OBJECT_RETURN_NONE,
+ 1, VM_PROT_NO_CHANGE, wait);
}
/* Have the kernel write back some pages of a pager from OFFSET to
OFFSET+SIZE; if WAIT is set, then wait for them to be finally
written before returning. */
-void
+error_t
pager_flush_some (struct pager *p, vm_address_t offset,
vm_size_t size, int wait)
{
- _pager_lock_object (p, offset, size, MEMORY_OBJECT_RETURN_NONE, 1,
- VM_PROT_NO_CHANGE, wait);
+ return _pager_lock_object (p, offset, size, MEMORY_OBJECT_RETURN_NONE,
+ 1, VM_PROT_NO_CHANGE, wait);
}
diff --git a/libpager/pager-return.c b/libpager/pager-return.c
index 7563d449..56b7937f 100644
--- a/libpager/pager-return.c
+++ b/libpager/pager-return.c
@@ -22,22 +22,25 @@
/* Have all dirty pages written back, and also flush the contents of
the kernel's cache. */
-void
+error_t
pager_return (struct pager *p, int wait)
{
+ error_t err;
vm_address_t offset;
vm_size_t len;
-
- pager_report_extent (p->upi, &offset, &len);
-
- _pager_lock_object (p, offset, len, MEMORY_OBJECT_RETURN_ALL, 1,
- VM_PROT_NO_CHANGE, wait);
+
+ err = pager_report_extent (p->upi, &offset, &len);
+ if (err)
+ return err;
+
+ return _pager_lock_object (p, offset, len, MEMORY_OBJECT_RETURN_ALL,
+ 1, VM_PROT_NO_CHANGE, wait);
}
-void
+error_t
pager_return_some (struct pager *p, vm_address_t offset,
vm_size_t size, int wait)
{
- _pager_lock_object (p, offset, size, MEMORY_OBJECT_RETURN_ALL, 1,
- VM_PROT_NO_CHANGE, wait);
+ return _pager_lock_object (p, offset, size, MEMORY_OBJECT_RETURN_ALL,
+ 1, VM_PROT_NO_CHANGE, wait);
}
diff --git a/libpager/pager-sync.c b/libpager/pager-sync.c
index ec024b6d..34439a7a 100644
--- a/libpager/pager-sync.c
+++ b/libpager/pager-sync.c
@@ -20,25 +20,28 @@
/* Have the kernel write back all dirty pages in the pager; if
WAIT is set, then wait for them to be finally written before
returning. */
-void
+error_t
pager_sync (struct pager *p, int wait)
{
+ error_t err;
vm_address_t offset;
vm_size_t len;
- pager_report_extent (p->upi, &offset, &len);
-
- _pager_lock_object (p, offset, len, MEMORY_OBJECT_RETURN_ALL, 0,
- VM_PROT_NO_CHANGE, wait);
+ err = pager_report_extent (p->upi, &offset, &len);
+ if (err)
+ return err;
+
+ return _pager_lock_object (p, offset, len, MEMORY_OBJECT_RETURN_ALL,
+ 0, VM_PROT_NO_CHANGE, wait);
}
/* Have the kernel write back some pages of a pager; if WAIT is set,
then wait for them to be finally written before returning. */
-void
+error_t
pager_sync_some (struct pager *p, vm_address_t offset,
vm_size_t size, int wait)
{
- _pager_lock_object (p, offset, size, MEMORY_OBJECT_RETURN_ALL, 0,
- VM_PROT_NO_CHANGE, wait);
+ return _pager_lock_object (p, offset, size, MEMORY_OBJECT_RETURN_ALL,
+ 0, VM_PROT_NO_CHANGE, wait);
}
diff --git a/libpager/pager.h b/libpager/pager.h
index d2a8d397..df7fbfc8 100644
--- a/libpager/pager.h
+++ b/libpager/pager.h
@@ -83,14 +83,14 @@ pager_get_upi (struct pager *p);
/* Sync data from pager PAGER to backing store; wait for
all the writes to complete iff WAIT is set. */
-void
+error_t
pager_sync (struct pager *pager,
int wait);
/* Sync some data (starting at START, for LEN bytes) from pager PAGER
to backing store. Wait for all the writes to complete iff WAIT is
set. */
-void
+error_t
pager_sync_some (struct pager *pager,
vm_address_t start,
vm_size_t len,
@@ -98,14 +98,14 @@ pager_sync_some (struct pager *pager,
/* Flush data from the kernel for pager PAGER and force any pending
delayed copies. Wait for all pages to be flushed iff WAIT is set. */
-void
+error_t
pager_flush (struct pager *pager,
int wait);
/* Flush some data (starting at START, for LEN bytes) for pager PAGER
from the kernel. Wait for all pages to be flushed iff WAIT is set. */
-void
+error_t
pager_flush_some (struct pager *pager,
vm_address_t start,
vm_size_t len,
@@ -114,15 +114,15 @@ pager_flush_some (struct pager *pager,
/* Flush data from the kernel for pager PAGER and force any pending
delayed copies. Wait for all pages to be flushed iff WAIT is set.
Have the kernel write back modifications. */
-void
+error_t
pager_return (struct pager *pager,
int wait);
/* Flush some data (starting at START, for LEN bytes) for pager PAGER
- from the kernel. Wait for all pages to be flushed iff WAIT is set.
+ from the kernel. Wait for all pages to be flushed iff WAIT is set.
Have the kernel write back modifications. */
-void
+error_t
pager_return_some (struct pager *pager,
vm_address_t start,
vm_size_t len,
@@ -132,18 +132,18 @@ pager_return_some (struct pager *pager,
page will be paged out at some future point, otherwise it might be
dropped by the kernel. If the page is currently in core, the
kernel might ignore this call. */
-void
+error_t
pager_offer_page (struct pager *pager,
int precious,
int writelock,
vm_offset_t page,
- vm_address_t buf);
+ vm_address_t buf);
/* Change the attributes of the memory object underlying pager PAGER.
Arguments MAY_CACHE and COPY_STRATEGY are as for
memory_object_change_attributes. Wait for the kernel to report
completion if WAIT is set. */
-void
+error_t
pager_change_attributes (struct pager *pager,
boolean_t may_cache,
memory_object_copy_strategy_t copy_strategy,
diff --git a/libpager/priv.h b/libpager/priv.h
index 5d6a07e6..c0a99fe3 100644
--- a/libpager/priv.h
+++ b/libpager/priv.h
@@ -138,8 +138,8 @@ void _pager_mark_next_request_error (struct pager *,
vm_address_t,
vm_size_t, error_t);
void _pager_mark_object_error (struct pager *, vm_address_t,
vm_size_t, error_t);
-void _pager_lock_object (struct pager *, vm_offset_t, vm_size_t, int, int,
- vm_prot_t, int);
+error_t _pager_lock_object (struct pager *, vm_offset_t, vm_size_t,
+ int, int, vm_prot_t, int);
void _pager_free_structure (struct pager *);
void _pager_clean (void *arg);
void _pager_real_dropweak (void *arg);
--
2.31.1
- [PATCH 1/6] libpager: Fix mixing up success and error, (continued)
- [PATCH 1/6] libpager: Fix mixing up success and error, Sergey Bugaev, 2021/05/06
- [PATCH 2/6] libpager: Do not flush in-core pages on offer, Sergey Bugaev, 2021/05/06
- [PATCH 4/6] libpager: Store pagemapsize as vm_size_t, Sergey Bugaev, 2021/05/06
- [PATCH 6/6] libpager: Use libc heap for pagemap, Sergey Bugaev, 2021/05/06
- [PATCH 3/6] libpager: Add error handling to various functions,
Sergey Bugaev <=
- [PATCH 5/6] libpager: Fix overallocating pagemap, Sergey Bugaev, 2021/05/06
- And another patch..., Sergey Bugaev, 2021/05/06
- Re: And another patch..., Samuel Thibault, 2021/05/08
- Re: And another patch..., Sergey Bugaev, 2021/05/08
- [PATCH 0/4] _pager_do_write_request fixes, Sergey Bugaev, 2021/05/08
- [PATCH 3/4] libpager: Track which pages are precious, Sergey Bugaev, 2021/05/08
- [PATCH 4/4] libpager: Do not throw away precious pages, Sergey Bugaev, 2021/05/08
- [PATCH 2/4] libpager: Store omit_data in an array, Sergey Bugaev, 2021/05/08
- [PATCH 1/4] libpager: pager_write_page () should not unmap page, Sergey Bugaev, 2021/05/08