From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
To: libc-alpha@sourceware.org
Subject: [PATCH 4/4] Move libio lock single-thread optimization to generic libc-lock (BZ #27842)
Date: Thu, 27 May 2021 17:44:57 -0300 [thread overview]
Message-ID: <20210527204457.1250196-5-adhemerval.zanella@linaro.org> (raw)
In-Reply-To: <20210527204457.1250196-1-adhemerval.zanella@linaro.org>
This patch moves the single thread stdio optimization (d2e04918833d9)
to the generic libc-lock. It simplifies the support, since there is
no need to add per-function code to handle the single-thread case,
and removes the _IO_enable_locks (the SINGLE_THREAD_P uses the either
the TCB or the global variable which indicates wether the process
is multi-threaded).
As for current approach, once the process goes multithreaded the
single-thread is disabled (it won't be enable back if the process
become single-threaded).
It also handles the memory streams requirement (de895ddcd7fc), since
the SINGLE_THREAD_P already contains te information whether the
process is multithread (so there is no need to disable the optimization
because such stream are listed in _IO_list_all).
Finally it also removed the flockfile uses a read-modify-write operation
on _flags2 outside a lock region (BZ #27842).
Checked on x86_64-linux-gnu, i686-linux-gnu, and aarch64-linux-gnu.
---
libio/Versions | 3 ---
libio/feof.c | 2 --
libio/ferror.c | 2 --
libio/fputc.c | 2 --
libio/genops.c | 28 ----------------------------
libio/getc.c | 2 --
libio/getchar.c | 4 +---
libio/iofopncook.c | 2 --
libio/ioungetc.c | 2 --
libio/libio.h | 4 ----
libio/memstream.c | 3 ---
libio/putc.c | 2 --
libio/wmemstream.c | 3 ---
nptl/pthread_create.c | 3 ---
stdio-common/flockfile.c | 1 -
sysdeps/mach/libc-lock.h | 9 +++++----
sysdeps/nptl/libc-lock.h | 7 ++++---
17 files changed, 10 insertions(+), 69 deletions(-)
diff --git a/libio/Versions b/libio/Versions
index 6f1ab96100..73852bd007 100644
--- a/libio/Versions
+++ b/libio/Versions
@@ -156,9 +156,6 @@ libc {
# Used by NPTL and librt
__libc_fatal;
- # Used by NPTL
- _IO_enable_locks;
-
__fseeko64;
__ftello64;
}
diff --git a/libio/feof.c b/libio/feof.c
index 2664b4caac..7b4e9a60e3 100644
--- a/libio/feof.c
+++ b/libio/feof.c
@@ -32,8 +32,6 @@ _IO_feof (FILE *fp)
{
int result;
CHECK_FILE (fp, EOF);
- if (!_IO_need_lock (fp))
- return _IO_feof_unlocked (fp);
_IO_flockfile (fp);
result = _IO_feof_unlocked (fp);
_IO_funlockfile (fp);
diff --git a/libio/ferror.c b/libio/ferror.c
index eef85c2602..dd3f0c8302 100644
--- a/libio/ferror.c
+++ b/libio/ferror.c
@@ -32,8 +32,6 @@ _IO_ferror (FILE *fp)
{
int result;
CHECK_FILE (fp, EOF);
- if (!_IO_need_lock (fp))
- return _IO_ferror_unlocked (fp);
_IO_flockfile (fp);
result = _IO_ferror_unlocked (fp);
_IO_funlockfile (fp);
diff --git a/libio/fputc.c b/libio/fputc.c
index 55877668c1..8abe5e80f0 100644
--- a/libio/fputc.c
+++ b/libio/fputc.c
@@ -32,8 +32,6 @@ fputc (int c, FILE *fp)
{
int result;
CHECK_FILE (fp, EOF);
- if (!_IO_need_lock (fp))
- return _IO_putc_unlocked (c, fp);
_IO_acquire_lock (fp);
result = _IO_putc_unlocked (c, fp);
_IO_release_lock (fp);
diff --git a/libio/genops.c b/libio/genops.c
index e501309b35..44b7ba9926 100644
--- a/libio/genops.c
+++ b/libio/genops.c
@@ -488,39 +488,11 @@ _IO_init (FILE *fp, int flags)
_IO_init_internal (fp, flags);
}
-static int stdio_needs_locking;
-
-/* In a single-threaded process most stdio locks can be omitted. After
- _IO_enable_locks is called, locks are not optimized away any more.
- It must be first called while the process is still single-threaded.
-
- This lock optimization can be disabled on a per-file basis by setting
- _IO_FLAGS2_NEED_LOCK, because a file can have user-defined callbacks
- or can be locked with flockfile and then a thread may be created
- between a lock and unlock, so omitting the lock is not valid.
-
- Here we have to make sure that the flag is set on all existing files
- and files created later. */
-void
-_IO_enable_locks (void)
-{
- _IO_ITER i;
-
- if (stdio_needs_locking)
- return;
- stdio_needs_locking = 1;
- for (i = _IO_iter_begin (); i != _IO_iter_end (); i = _IO_iter_next (i))
- _IO_iter_file (i)->_flags2 |= _IO_FLAGS2_NEED_LOCK;
-}
-libc_hidden_def (_IO_enable_locks)
-
void
_IO_old_init (FILE *fp, int flags)
{
fp->_flags = _IO_MAGIC|flags;
fp->_flags2 = 0;
- if (stdio_needs_locking)
- fp->_flags2 |= _IO_FLAGS2_NEED_LOCK;
fp->_IO_buf_base = NULL;
fp->_IO_buf_end = NULL;
fp->_IO_read_base = NULL;
diff --git a/libio/getc.c b/libio/getc.c
index bf67362cd9..78709bffa8 100644
--- a/libio/getc.c
+++ b/libio/getc.c
@@ -34,8 +34,6 @@ _IO_getc (FILE *fp)
{
int result;
CHECK_FILE (fp, EOF);
- if (!_IO_need_lock (fp))
- return _IO_getc_unlocked (fp);
_IO_acquire_lock (fp);
result = _IO_getc_unlocked (fp);
_IO_release_lock (fp);
diff --git a/libio/getchar.c b/libio/getchar.c
index 5c7307e392..270ccc0905 100644
--- a/libio/getchar.c
+++ b/libio/getchar.c
@@ -33,10 +33,8 @@ int
getchar (void)
{
int result;
- if (!_IO_need_lock (stdin))
- return _IO_getc_unlocked (stdin);
_IO_acquire_lock (stdin);
result = _IO_getc_unlocked (stdin);
_IO_release_lock (stdin);
return result;
-}
\ No newline at end of file
+}
diff --git a/libio/iofopncook.c b/libio/iofopncook.c
index 57285cc291..b6761f1e25 100644
--- a/libio/iofopncook.c
+++ b/libio/iofopncook.c
@@ -162,8 +162,6 @@ _IO_cookie_init (struct _IO_cookie_file *cfile, int read_write,
_IO_mask_flags (&cfile->__fp.file, read_write,
_IO_NO_READS+_IO_NO_WRITES+_IO_IS_APPENDING);
- cfile->__fp.file._flags2 |= _IO_FLAGS2_NEED_LOCK;
-
/* We use a negative number different from -1 for _fileno to mark that
this special stream is not associated with a real file, but still has
to be treated as such. */
diff --git a/libio/ioungetc.c b/libio/ioungetc.c
index dcf1c4ca9e..9c149f15d3 100644
--- a/libio/ioungetc.c
+++ b/libio/ioungetc.c
@@ -33,8 +33,6 @@ _IO_ungetc (int c, FILE *fp)
CHECK_FILE (fp, EOF);
if (c == EOF)
return EOF;
- if (!_IO_need_lock (fp))
- return _IO_sputbackc (fp, (unsigned char) c);
_IO_acquire_lock (fp);
result = _IO_sputbackc (fp, (unsigned char) c);
_IO_release_lock (fp);
diff --git a/libio/libio.h b/libio/libio.h
index bd42479f13..60431e726d 100644
--- a/libio/libio.h
+++ b/libio/libio.h
@@ -89,7 +89,6 @@ typedef struct
#define _IO_FLAGS2_USER_WBUF 8
#define _IO_FLAGS2_NOCLOSE 32
#define _IO_FLAGS2_CLOEXEC 64
-#define _IO_FLAGS2_NEED_LOCK 128
/* _IO_pos_BAD is an off64_t value indicating error, unknown, or EOF. */
#define _IO_pos_BAD ((off64_t) -1)
@@ -213,9 +212,6 @@ extern int _IO_ftrylockfile (FILE *) __THROW;
#define _IO_cleanup_region_end(_Doit) /**/
#endif
-#define _IO_need_lock(_fp) \
- (((_fp)->_flags2 & _IO_FLAGS2_NEED_LOCK) != 0)
-
extern int _IO_vfscanf (FILE * __restrict, const char * __restrict,
__gnuc_va_list, int *__restrict);
extern __ssize_t _IO_padn (FILE *, int, __ssize_t);
diff --git a/libio/memstream.c b/libio/memstream.c
index 6763689038..368c23f7e8 100644
--- a/libio/memstream.c
+++ b/libio/memstream.c
@@ -92,9 +92,6 @@ __open_memstream (char **bufloc, size_t *sizeloc)
new_f->fp.bufloc = bufloc;
new_f->fp.sizeloc = sizeloc;
- /* Disable single thread optimization. BZ 21735. */
- new_f->fp._sf._sbf._f._flags2 |= _IO_FLAGS2_NEED_LOCK;
-
return (FILE *) &new_f->fp._sf._sbf;
}
libc_hidden_def (__open_memstream)
diff --git a/libio/putc.c b/libio/putc.c
index bc9dea856d..72e7ec3f0b 100644
--- a/libio/putc.c
+++ b/libio/putc.c
@@ -25,8 +25,6 @@ _IO_putc (int c, FILE *fp)
{
int result;
CHECK_FILE (fp, EOF);
- if (!_IO_need_lock (fp))
- return _IO_putc_unlocked (c, fp);
_IO_acquire_lock (fp);
result = _IO_putc_unlocked (c, fp);
_IO_release_lock (fp);
diff --git a/libio/wmemstream.c b/libio/wmemstream.c
index 037349c83c..d441c4248c 100644
--- a/libio/wmemstream.c
+++ b/libio/wmemstream.c
@@ -94,9 +94,6 @@ open_wmemstream (wchar_t **bufloc, size_t *sizeloc)
new_f->fp.bufloc = bufloc;
new_f->fp.sizeloc = sizeloc;
- /* Disable single thread optimization. BZ 21735. */
- new_f->fp._sf._sbf._f._flags2 |= _IO_FLAGS2_NEED_LOCK;
-
return (FILE *) &new_f->fp._sf._sbf;
}
diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index 2d2535b07d..d856c7bce3 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -718,9 +718,6 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr,
collect_default_sched (pd);
}
- if (__glibc_unlikely (__nptl_nthreads == 1))
- _IO_enable_locks ();
-
/* Pass the descriptor to the caller. */
*newthread = (pthread_t) pd;
diff --git a/stdio-common/flockfile.c b/stdio-common/flockfile.c
index a66e0a731e..da382aebdf 100644
--- a/stdio-common/flockfile.c
+++ b/stdio-common/flockfile.c
@@ -22,7 +22,6 @@
void
__flockfile (FILE *stream)
{
- stream->_flags2 |= _IO_FLAGS2_NEED_LOCK;
_IO_lock_lock (*stream->_lock);
}
weak_alias (__flockfile, flockfile);
diff --git a/sysdeps/mach/libc-lock.h b/sysdeps/mach/libc-lock.h
index 220fcee921..7b20d45470 100644
--- a/sysdeps/mach/libc-lock.h
+++ b/sysdeps/mach/libc-lock.h
@@ -106,9 +106,9 @@ typedef struct __libc_lock_recursive_opaque__ __libc_lock_recursive_t;
__libc_lock_recursive_t *const __lock = &(NAME); \
void *__self = __libc_lock_owner_self (); \
int __r = 0; \
- if (__self == __lock->owner) \
+ if (!SINGLE_THREAD_P && __self == __lock->owner) \
++__lock->cnt; \
- else if ((__r = lll_trylock (__lock->lock)) == 0) \
+ else if (!SINGLE_THREAD_P && (__r = lll_trylock (__lock->lock)) == 0) \
__lock->owner = __self, __lock->cnt = 1; \
__r; \
})
@@ -117,7 +117,7 @@ typedef struct __libc_lock_recursive_opaque__ __libc_lock_recursive_t;
({ \
__libc_lock_recursive_t *const __lock = &(NAME); \
void *__self = __libc_lock_owner_self (); \
- if (__self != __lock->owner) \
+ if (!SINGLE_THREAD_P && __self != __lock->owner) \
{ \
lll_lock (__lock->lock, 0); \
__lock->owner = __self; \
@@ -132,7 +132,8 @@ typedef struct __libc_lock_recursive_opaque__ __libc_lock_recursive_t;
if (--__lock->cnt == 0) \
{ \
__lock->owner = 0; \
- lll_unlock (__lock->lock, 0); \
+ if (!SINGLE_THREAD_P) \
+ lll_unlock (__lock->lock, 0); \
} \
})
diff --git a/sysdeps/nptl/libc-lock.h b/sysdeps/nptl/libc-lock.h
index 627b13309a..fd5ab91abf 100644
--- a/sysdeps/nptl/libc-lock.h
+++ b/sysdeps/nptl/libc-lock.h
@@ -90,7 +90,7 @@ typedef struct __libc_lock_recursive_opaque__ __libc_lock_recursive_t;
# define __libc_lock_lock_recursive(NAME) \
do { \
void *self = THREAD_SELF; \
- if ((NAME).owner != self) \
+ if (!SINGLE_THREAD_P && (NAME).owner != self) \
{ \
lll_lock ((NAME).lock, LLL_PRIVATE); \
(NAME).owner = self; \
@@ -108,7 +108,7 @@ typedef struct __libc_lock_recursive_opaque__ __libc_lock_recursive_t;
({ \
int result = 0; \
void *self = THREAD_SELF; \
- if ((NAME).owner != self) \
+ if (!SINGLE_THREAD_P && (NAME).owner != self) \
{ \
if (lll_trylock ((NAME).lock) == 0) \
{ \
@@ -135,7 +135,8 @@ typedef struct __libc_lock_recursive_opaque__ __libc_lock_recursive_t;
if (--(NAME).cnt == 0) \
{ \
(NAME).owner = NULL; \
- lll_unlock ((NAME).lock, LLL_PRIVATE); \
+ if (!SINGLE_THREAD_P) \
+ lll_unlock ((NAME).lock, LLL_PRIVATE); \
} \
} while (0)
#else
--
2.30.2
next prev parent reply other threads:[~2021-05-27 20:45 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-27 20:44 [PATCH 0/4] Move libio lock single-thread optimization to generic libc-lock Adhemerval Zanella
2021-05-27 20:44 ` [PATCH 1/4] libio: Assume _IO_lock_inexpensive Adhemerval Zanella
2021-05-30 13:11 ` Florian Weimer
2021-05-27 20:44 ` [PATCH 2/4] libio: Assume _IO_MTSAFE_IO Adhemerval Zanella
2021-05-27 21:38 ` Andreas Schwab
2021-05-27 20:44 ` [PATCH 3/4] Consolidate stdio-lock.h Adhemerval Zanella
2021-05-27 20:44 ` Adhemerval Zanella [this message]
2022-04-12 12:18 ` [PATCH 0/4] Move libio lock single-thread optimization to generic libc-lock Florian Weimer
2022-04-14 14:23 ` Adhemerval Zanella
2022-04-14 15:28 ` Florian Weimer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210527204457.1250196-5-adhemerval.zanella@linaro.org \
--to=adhemerval.zanella@linaro.org \
--cc=libc-alpha@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).