public inbox for [email protected]
help / color / mirror / Atom feedFrom: Zhang Mingli <[email protected]>
To: Richard Guo <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Fix condition in shm_toc and remove unused function shm_toc_freespace.
Date: Fri, 13 Jan 2023 23:09:30 +0800
Message-ID: <0c84a55b-9b30-40d1-873a-873ee134c04e@Spark> (raw)
In-Reply-To: <CAMbWs49f3M1ZrFuiQE5AoWqTERUUXdoeKFg6Di6BuYWFSZ24pA@mail.gmail.com>
References: <f937934f-5b6c-402c-a9ab-605bd07dcba9@Spark>
<1c7954e4-1705-4855-a99d-c77bf426c3e7@Spark>
<70ce9710-e9ed-4a25-a327-88de830b8cf6@Spark>
<CAMbWs49f3M1ZrFuiQE5AoWqTERUUXdoeKFg6Di6BuYWFSZ24pA@mail.gmail.com>
Hi,
Regards,
Zhang Mingli
On Jan 12, 2023, 16:54 +0800, Richard Guo <[email protected]>, wrote:
>
> On Thu, Jan 12, 2023 at 2:50 PM Zhang Mingli <[email protected]> wrote:
> > On Jan 12, 2023, 14:34 +0800, Zhang Mingli <[email protected]>, wrote:
> > > Some conditions in shm_toc_insert and shm_toc_allocate are bogus, like:
> > > if (toc_bytes + nbytes > total_bytes || toc_bytes + nbytes < toc_bytes)Remove the condition `toc_bytes + nbytes < toc_bytes` and take a sizeof(shm_entry) into account in shm_toc_allocate though
> > > shm_toc_allocate does that too.
> > shm_toc_insert does that too, and we can report error earlier.
>
> I don't think we should consider sizeof(shm_toc_entry) in the 'if'
> condition in shm_toc_allocate, as this function is not in charge of
> allocating a new TOC entry. That's what shm_toc_insert does.
Thanks for review.
Make sense.
Even reserve a sizeof(shm_toc_entry) when shm_toc_allocate, it cloud happen that there is no space when shm_toc_insert
in case of other processes may take space after that.
Patch updated.
Attachments:
[application/octet-stream] v1-0001-Fix-condition-in-shm_toc-and-remove-unused-functi.patch (2.7K, ../0c84a55b-9b30-40d1-873a-873ee134c04e@Spark/3-v1-0001-Fix-condition-in-shm_toc-and-remove-unused-functi.patch)
download | inline diff:
From 766d20b666b75ef74767d2e9e3cf3c872c4e7521 Mon Sep 17 00:00:00 2001
From: Zhang Mingli <[email protected]>
Date: Thu, 12 Jan 2023 14:11:43 +0800
Subject: [PATCH v1] Fix condition in shm_toc and remove unused function
shm_toc_freespace.
Fix bogus condition in shm_toc_insert and shm_toc_allocate.
Remove unused function shm_toc_freespace.
---
src/backend/storage/ipc/shm_toc.c | 26 +-------------------------
src/include/storage/shm_toc.h | 1 -
2 files changed, 1 insertion(+), 26 deletions(-)
diff --git a/src/backend/storage/ipc/shm_toc.c b/src/backend/storage/ipc/shm_toc.c
index 0cd824400f..ed16947d4c 100644
--- a/src/backend/storage/ipc/shm_toc.c
+++ b/src/backend/storage/ipc/shm_toc.c
@@ -110,7 +110,7 @@ shm_toc_allocate(shm_toc *toc, Size nbytes)
+ allocated_bytes;
/* Check for memory exhaustion and overflow. */
- if (toc_bytes + nbytes > total_bytes || toc_bytes + nbytes < toc_bytes)
+ if (toc_bytes + nbytes > total_bytes)
{
SpinLockRelease(&toc->toc_mutex);
ereport(ERROR,
@@ -124,29 +124,6 @@ shm_toc_allocate(shm_toc *toc, Size nbytes)
return ((char *) toc) + (total_bytes - allocated_bytes - nbytes);
}
-/*
- * Return the number of bytes that can still be allocated.
- */
-Size
-shm_toc_freespace(shm_toc *toc)
-{
- volatile shm_toc *vtoc = toc;
- Size total_bytes;
- Size allocated_bytes;
- Size nentry;
- Size toc_bytes;
-
- SpinLockAcquire(&toc->toc_mutex);
- total_bytes = vtoc->toc_total_bytes;
- allocated_bytes = vtoc->toc_allocated_bytes;
- nentry = vtoc->toc_nentry;
- SpinLockRelease(&toc->toc_mutex);
-
- toc_bytes = offsetof(shm_toc, toc_entry) + nentry * sizeof(shm_toc_entry);
- Assert(allocated_bytes + BUFFERALIGN(toc_bytes) <= total_bytes);
- return total_bytes - (allocated_bytes + BUFFERALIGN(toc_bytes));
-}
-
/*
* Insert a TOC entry.
*
@@ -191,7 +168,6 @@ shm_toc_insert(shm_toc *toc, uint64 key, void *address)
/* Check for memory exhaustion and overflow. */
if (toc_bytes + sizeof(shm_toc_entry) > total_bytes ||
- toc_bytes + sizeof(shm_toc_entry) < toc_bytes ||
nentry >= PG_UINT32_MAX)
{
SpinLockRelease(&toc->toc_mutex);
diff --git a/src/include/storage/shm_toc.h b/src/include/storage/shm_toc.h
index 7a2f8e9934..5f2abcc613 100644
--- a/src/include/storage/shm_toc.h
+++ b/src/include/storage/shm_toc.h
@@ -30,7 +30,6 @@ typedef struct shm_toc shm_toc;
extern shm_toc *shm_toc_create(uint64 magic, void *address, Size nbytes);
extern shm_toc *shm_toc_attach(uint64 magic, void *address);
extern void *shm_toc_allocate(shm_toc *toc, Size nbytes);
-extern Size shm_toc_freespace(shm_toc *toc);
extern void shm_toc_insert(shm_toc *toc, uint64 key, void *address);
extern void *shm_toc_lookup(shm_toc *toc, uint64 key, bool noError);
--
2.34.1
view thread (5+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: Fix condition in shm_toc and remove unused function shm_toc_freespace.
In-Reply-To: <0c84a55b-9b30-40d1-873a-873ee134c04e@Spark>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox