Page MenuHomeFreeBSD

Contributor Reviews (src)Project
ActivePublic

Recent Activity

Sat, Sep 9

minsoochoo0122_proton.me added a comment to D41421: Update jemalloc to version 5.3.0.

Parent revision D41461 is closed. Now we can test this on local machines on -CURRENT.

I've been testing this on main and stable/13 (with some tweaks). If there are any specific tests I can do that would be helpful let me know

In the release notes, there are some new features added in jemalloc 5.3.0. For example,

  • Make the behavior of realloc(ptr, 0) configurable with opt.zero_realloc
  • Add the thread.idle mallctl which hints that the calling thread will be idle for a nontrivial period of time.
  • Add mallctl interfaces:
    • opt.zero_realloc (@davidtgoldblatt)
    • opt.cache_oblivious (@interwq)
    • opt.prof_leak_error (@yunxuo)
    • opt.stats_interval (@interwq)
    • opt.stats_interval_opts (@interwq)
    • opt.tcache_max (@interwq)
    • opt.trust_madvise (@azat)
    • prof.prefix (@zhxchen17)
    • stats.zero_reallocs (@davidtgoldblatt)
    • thread.idle (@davidtgoldblatt)
    • thread.peak.{read,reset} (@davidtgoldblatt)

and so on.

It would be really nice if we can test this against with the tests under /usr/tests . I know it is still very tricky to run it. If you are interested, please check the scripts at https://github.com/freebsd/freebsd-ci and let me know (and/or -testing mailing list) if you have any questions.

By the way, because this is not a major version bump (5.x.x), will this version bump be abled to be MFCed to stable/14?

Theoretically, yes, but it would be nice to check the compatibility thoroughly.

Sat, Sep 9, 5:29 PM · Contributor Reviews (src)
minsoochoo0122_proton.me added inline comments to D41421: Update jemalloc to version 5.3.0.
Sat, Sep 9, 12:59 AM · Contributor Reviews (src)

Fri, Sep 8

jrtc27 added inline comments to D41421: Update jemalloc to version 5.3.0.
Fri, Sep 8, 1:29 AM · Contributor Reviews (src)
minsoochoo0122_proton.me updated the diff for D41421: Update jemalloc to version 5.3.0.
  • Use memalign from jemalloc
Fri, Sep 8, 1:19 AM · Contributor Reviews (src)

Wed, Sep 6

brooks added a comment to D41421: Update jemalloc to version 5.3.0.

I think it might make sense to split the switch to the jemalloc memalign() into a separate commit.

Wed, Sep 6, 8:03 PM · Contributor Reviews (src)

Mon, Sep 4

lwhsu added a comment to D41421: Update jemalloc to version 5.3.0.

Parent revision D41461 is closed. Now we can test this on local machines on -CURRENT.

I've been testing this on main and stable/13 (with some tweaks). If there are any specific tests I can do that would be helpful let me know

In the release notes, there are some new features added in jemalloc 5.3.0. For example,

  • Make the behavior of realloc(ptr, 0) configurable with opt.zero_realloc
  • Add the thread.idle mallctl which hints that the calling thread will be idle for a nontrivial period of time.
  • Add mallctl interfaces:
    • opt.zero_realloc (@davidtgoldblatt)
    • opt.cache_oblivious (@interwq)
    • opt.prof_leak_error (@yunxuo)
    • opt.stats_interval (@interwq)
    • opt.stats_interval_opts (@interwq)
    • opt.tcache_max (@interwq)
    • opt.trust_madvise (@azat)
    • prof.prefix (@zhxchen17)
    • stats.zero_reallocs (@davidtgoldblatt)
    • thread.idle (@davidtgoldblatt)
    • thread.peak.{read,reset} (@davidtgoldblatt)

and so on.

Mon, Sep 4, 5:40 PM · Contributor Reviews (src)

Sat, Sep 2

minsoochoo0122_proton.me added a comment to D41421: Update jemalloc to version 5.3.0.

Parent revision D41461 is closed. Now we can test this on local machines on -CURRENT.

I've been testing this on main and stable/13 (with some tweaks). If there are any specific tests I can do that would be helpful let me know

Sat, Sep 2, 3:09 AM · Contributor Reviews (src)

Aug 26 2023

instructionset_gmail.com added a comment to D41421: Update jemalloc to version 5.3.0.

Parent revision D41461 is closed. Now we can test this on local machines on -CURRENT.

Aug 26 2023, 5:58 AM · Contributor Reviews (src)

Aug 21 2023

minsoochoo0122_proton.me added a comment to D41421: Update jemalloc to version 5.3.0.

Parent revision D41461 is closed. Now we can test this on local machines on -CURRENT.

Aug 21 2023, 1:48 AM · Contributor Reviews (src)

Aug 20 2023

kib closed D41461: libc: make pthread_getname_np and _pthread_getname_np available.
Aug 20 2023, 10:45 PM · Contributor Reviews (src)
kib added a comment to D41461: libc: make pthread_getname_np and _pthread_getname_np available.

You may send me the complete git patch by mail. Please ensure that the author metadata is correctly filled. Also it would be useful to add tags like Differential revision: etc.

Aug 20 2023, 9:26 PM · Contributor Reviews (src)
kib accepted D41461: libc: make pthread_getname_np and _pthread_getname_np available.
Aug 20 2023, 9:24 PM · Contributor Reviews (src)
minsoochoo0122_proton.me updated the diff for D41461: libc: make pthread_getname_np and _pthread_getname_np available.

Stores null string in name parameter for stub function.

Aug 20 2023, 9:19 PM · Contributor Reviews (src)
minsoochoo0122_proton.me added a comment to D41461: libc: make pthread_getname_np and _pthread_getname_np available.
In D41461#945982, @kib wrote:

Is there any possible way to store null string in _pthread_stubs.c? I can't figure out how to store null string using STUB_FUNC3.

I do not understand neither your questions, nor can I guess the possible confusion behind them. Would it be simpler if I provide the compilable (not tested) changes for _pthread_stub.c?

diff --git a/lib/libc/gen/_pthread_stubs.c b/lib/libc/gen/_pthread_stubs.c
index 6741c6a5ec51..3fc0817b8655 100644
--- a/lib/libc/gen/_pthread_stubs.c
+++ b/lib/libc/gen/_pthread_stubs.c
@@ -58,6 +58,7 @@ static int		stub_fail(void);
 static int		stub_true(void);
 static void		stub_exit(void);
 static int		stub_esrch(void);
+static int		stub_getname_np(pthread_t, char *, size_t);
 
 #define	PJT_DUAL_ENTRY(entry)	\
 	(pthread_func_t)entry, (pthread_func_t)entry
@@ -131,6 +132,7 @@ pthread_func_entry_t __thr_jtable[PJT_MAX] = {
 	[PJT_MUTEXATTR_SETROBUST] =	{PJT_DUAL_ENTRY(stub_zero)},
 	[PJT_GETTHREADID_NP] =		{PJT_DUAL_ENTRY(stub_zero)},
 	[PJT_ATTR_GET_NP] =		{PJT_DUAL_ENTRY(stub_esrch)},
+	[PJT_GETNAME_NP] =		{PJT_DUAL_ENTRY(stub_getname_np)},
 };
 
 /*
@@ -289,6 +291,7 @@ STUB_FUNC3(__pthread_cleanup_push_imp, PJT_CLEANUP_PUSH_IMP, void, void *,
 STUB_FUNC1(_pthread_cancel_enter, PJT_CANCEL_ENTER, void, int)
 STUB_FUNC1(_pthread_cancel_leave, PJT_CANCEL_LEAVE, void, int)
 STUB_FUNC2(pthread_attr_get_np, PJT_ATTR_GET_NP, int, pthread_t, pthread_attr_t *)
+STUB_FUNC3(pthread_gettname_np,	PJT_GETNAME_NP, int, pthread_t, char *, size_t)
 
 static int
 stub_zero(void)
@@ -337,3 +340,13 @@ stub_esrch(void)
 {
 	return (ESRCH);
 }
+
+static int
+stub_getname_np(pthread_t thread, char *buf, size_t len)
+{
+	if (thread != &main_thread)
+		return (ESRCH);
+	if (len >= 1)
+		buf[0] = '\0';
+	return (0);
+}
Aug 20 2023, 9:18 PM · Contributor Reviews (src)
kib added a comment to D41461: libc: make pthread_getname_np and _pthread_getname_np available.

Is there any possible way to store null string in _pthread_stubs.c? I can't figure out how to store null string using STUB_FUNC3.

I do not understand neither your questions, nor can I guess the possible confusion behind them. Would it be simpler if I provide the compilable (not tested) changes for _pthread_stub.c?

Aug 20 2023, 6:44 PM · Contributor Reviews (src)
minsoochoo0122_proton.me added a comment to D41461: libc: make pthread_getname_np and _pthread_getname_np available.
In D41461#945957, @kib wrote:
In D41461#945942, @kib wrote:
In D41461#945937, @kib wrote:

This is still wrong IMO. The main case is to return empty name for main thread, and it does not matter much if we do not detect invalid thread handle.

As was said before, stub pthread_getname_np() should store nul string in the target buffer.

How can I store null string to the name parameter? Does stub_null works for parameters as well?

You do if (len >= 1) name[0] = '\0'; as usual. What is the problem? stub_null() cannot be used, of course.

Sorry, my question was unclear. In `lib/libthr/thread/thr_info.c", I see

if (len > 0)
    buf[0] = '\0';

But I cannot find any file to write this for the stub function. Where can I write this code for stub functions? Like lib/libc/gen/__pthread_mutex_init_calloc_cb_stub.c, should I implement the stub function in a new file?

What is wrong with libc/gen/_pthread_stubs.c?

Example:

/* lib/libc/gen/__pthread_getname_np_stub.c */
#include <sys/cdefs.h>
#include <pthread.h>
#include <errno.h>
#include "libc_private.h"

int
_pthread_getname_np_stub(pthread_t thread, char *buf, size_t len)
{
    if (len > 0)
        buf[0] = '\0';
    return (ESRCH);
This should be `return (0);`

}

Aug 20 2023, 6:08 PM · Contributor Reviews (src)
kib added a comment to D41461: libc: make pthread_getname_np and _pthread_getname_np available.
In D41461#945942, @kib wrote:
In D41461#945937, @kib wrote:

This is still wrong IMO. The main case is to return empty name for main thread, and it does not matter much if we do not detect invalid thread handle.

As was said before, stub pthread_getname_np() should store nul string in the target buffer.

How can I store null string to the name parameter? Does stub_null works for parameters as well?

You do if (len >= 1) name[0] = '\0'; as usual. What is the problem? stub_null() cannot be used, of course.

Sorry, my question was unclear. In `lib/libthr/thread/thr_info.c", I see

if (len > 0)
    buf[0] = '\0';

But I cannot find any file to write this for the stub function. Where can I write this code for stub functions? Like lib/libc/gen/__pthread_mutex_init_calloc_cb_stub.c, should I implement the stub function in a new file?

What is wrong with libc/gen/_pthread_stubs.c?

Aug 20 2023, 5:36 PM · Contributor Reviews (src)
minsoochoo0122_proton.me added a comment to D41461: libc: make pthread_getname_np and _pthread_getname_np available.
In D41461#945942, @kib wrote:
In D41461#945937, @kib wrote:

This is still wrong IMO. The main case is to return empty name for main thread, and it does not matter much if we do not detect invalid thread handle.

As was said before, stub pthread_getname_np() should store nul string in the target buffer.

How can I store null string to the name parameter? Does stub_null works for parameters as well?

You do if (len >= 1) name[0] = '\0'; as usual. What is the problem? stub_null() cannot be used, of course.

Aug 20 2023, 5:05 PM · Contributor Reviews (src)
kib added a comment to D41461: libc: make pthread_getname_np and _pthread_getname_np available.
In D41461#945937, @kib wrote:

This is still wrong IMO. The main case is to return empty name for main thread, and it does not matter much if we do not detect invalid thread handle.

As was said before, stub pthread_getname_np() should store nul string in the target buffer.

How can I store null string to the name parameter? Does stub_null works for parameters as well?

Aug 20 2023, 3:56 PM · Contributor Reviews (src)
minsoochoo0122_proton.me added a comment to D41461: libc: make pthread_getname_np and _pthread_getname_np available.
In D41461#945937, @kib wrote:

This is still wrong IMO. The main case is to return empty name for main thread, and it does not matter much if we do not detect invalid thread handle.

As was said before, stub pthread_getname_np() should store nul string in the target buffer.

Aug 20 2023, 3:50 PM · Contributor Reviews (src)
kib added a comment to D41461: libc: make pthread_getname_np and _pthread_getname_np available.

This is still wrong IMO. The main case is to return empty name for main thread, and it does not matter much if we do not detect invalid thread handle.

Aug 20 2023, 3:23 PM · Contributor Reviews (src)
minsoochoo0122_proton.me updated the diff for D41461: libc: make pthread_getname_np and _pthread_getname_np available.

Using stub_esrch when an invalid thread comes a parameter.

Aug 20 2023, 2:14 AM · Contributor Reviews (src)

Aug 17 2023

emaste added a reviewer for D41492: Remove empty header annotations in Symbol.map files: imp.
Aug 17 2023, 2:56 PM · Contributor Reviews (src)
minsoochoo0122_proton.me requested review of D41492: Remove empty header annotations in Symbol.map files.
Aug 17 2023, 2:06 PM · Contributor Reviews (src)
minsoochoo0122_proton.me planned changes to D41492: Remove empty header annotations in Symbol.map files.
Aug 17 2023, 2:06 PM · Contributor Reviews (src)
minsoochoo0122_proton.me requested review of D41492: Remove empty header annotations in Symbol.map files.
Aug 17 2023, 2:05 PM · Contributor Reviews (src)
minsoochoo0122_proton.me updated the diff for D41421: Update jemalloc to version 5.3.0.

Updating diff file to conform recent changes in lib/libc/stdlib/malloc

Aug 17 2023, 2:05 AM · Contributor Reviews (src)
minsoochoo0122_proton.me added a comment to D41461: libc: make pthread_getname_np and _pthread_getname_np available.

pthread_getname_np returns 0 for success otherwise error code.

You didn't change the implementation, just the prototype? It's still not writing to the string, and it's reporting success even for garbage input threads.

Sorry, I'm new to contributing base system, and I don't understand which implementation you mean. From the code I read, pthread_getname_np is symbol of _thr_getname_np defined in lib/libthr/thread/thr_info.c, and it exports to libc. Is there any addition work to be done in stub? Thanks.

The stub needs to be a minimal implementation when there are no threads that satisfies the API documentation in the manpage. stub_zero does not do that, because it (a) doesn't validate the thread (b) doesn't initialise the buffer. This means that (a) if a caller provides junk to the function it returns 0 for success rather than ESRCH for failure (b) if a caller tries to use the buffer even after it gave it a pthread_t of &main_thread it'll be reading uninitialised junk (and may run off the end of the buffer if there doesn't happen to be a 0 byte in there). In short, stub_zero never works properly to implement pthread_getname_np.

Aug 17 2023, 12:11 AM · Contributor Reviews (src)

Aug 16 2023

jrtc27 added a comment to D41461: libc: make pthread_getname_np and _pthread_getname_np available.

pthread_getname_np returns 0 for success otherwise error code.

You didn't change the implementation, just the prototype? It's still not writing to the string, and it's reporting success even for garbage input threads.

Sorry, I'm new to contributing base system, and I don't understand which implementation you mean. From the code I read, pthread_getname_np is symbol of _thr_getname_np defined in lib/libthr/thread/thr_info.c, and it exports to libc. Is there any addition work to be done in stub? Thanks.

Aug 16 2023, 11:47 PM · Contributor Reviews (src)
minsoochoo0122_proton.me added a comment to D41461: libc: make pthread_getname_np and _pthread_getname_np available.

pthread_getname_np returns 0 for success otherwise error code.

You didn't change the implementation, just the prototype? It's still not writing to the string, and it's reporting success even for garbage input threads.

Aug 16 2023, 11:43 PM · Contributor Reviews (src)
jrtc27 added a comment to D41461: libc: make pthread_getname_np and _pthread_getname_np available.

pthread_getname_np returns 0 for success otherwise error code.

Aug 16 2023, 11:23 PM · Contributor Reviews (src)
minsoochoo0122_proton.me updated the diff for D41461: libc: make pthread_getname_np and _pthread_getname_np available.

pthread_getname_np returns 0 for success otherwise error code.

Aug 16 2023, 11:22 PM · Contributor Reviews (src)
minsoochoo0122_proton.me added inline comments to D41461: libc: make pthread_getname_np and _pthread_getname_np available.
Aug 16 2023, 9:58 PM · Contributor Reviews (src)
kib added inline comments to D41461: libc: make pthread_getname_np and _pthread_getname_np available.
Aug 16 2023, 7:30 PM · Contributor Reviews (src)
jrtc27 added inline comments to D41461: libc: make pthread_getname_np and _pthread_getname_np available.
Aug 16 2023, 6:54 PM · Contributor Reviews (src)
kib added inline comments to D41461: libc: make pthread_getname_np and _pthread_getname_np available.
Aug 16 2023, 6:43 PM · Contributor Reviews (src)
minsoochoo0122_proton.me updated the diff for D41461: libc: make pthread_getname_np and _pthread_getname_np available.

Don't export _pthread_getname_np

Aug 16 2023, 6:05 PM · Contributor Reviews (src)
kib added inline comments to D41461: libc: make pthread_getname_np and _pthread_getname_np available.
Aug 16 2023, 5:36 PM · Contributor Reviews (src)
minsoochoo0122_proton.me updated the diff for D41421: Update jemalloc to version 5.3.0.

Removed gnu11 flag and fixed gnu99 incompatibility

Aug 16 2023, 2:07 AM · Contributor Reviews (src)

Aug 15 2023

minsoochoo0122_proton.me added inline comments to D41461: libc: make pthread_getname_np and _pthread_getname_np available.
Aug 15 2023, 8:01 PM · Contributor Reviews (src)
kib added inline comments to D41461: libc: make pthread_getname_np and _pthread_getname_np available.
Aug 15 2023, 4:54 PM · Contributor Reviews (src)
minsoochoo0122_proton.me added inline comments to D41461: libc: make pthread_getname_np and _pthread_getname_np available.
Aug 15 2023, 12:27 PM · Contributor Reviews (src)
minsoochoo0122_proton.me added inline comments to D41461: libc: make pthread_getname_np and _pthread_getname_np available.
Aug 15 2023, 12:26 PM · Contributor Reviews (src)
kib added inline comments to D41461: libc: make pthread_getname_np and _pthread_getname_np available.
Aug 15 2023, 10:25 AM · Contributor Reviews (src)
minsoochoo0122_proton.me updated the diff for D41461: libc: make pthread_getname_np and _pthread_getname_np available.

pthread_getname_np goes to FBSD_1.6 to match version in pthread.map
pthread_getname_np STUB_FUNC3 not STUB_FUNC2 since it takes 3 parameters

Aug 15 2023, 6:23 AM · Contributor Reviews (src)
minsoochoo0122_proton.me added a comment to D41461: libc: make pthread_getname_np and _pthread_getname_np available.
In D41461#944269, @imp wrote:

I wonder why these were not exported... Is there any indication as to why? Was it intentoinal or an oversight?

Aug 15 2023, 5:24 AM · Contributor Reviews (src)
jrtc27 added a comment to D41461: libc: make pthread_getname_np and _pthread_getname_np available.
In D41461#944269, @imp wrote:

I wonder why these were not exported... Is there any indication as to why? Was it intentoinal or an oversight?

Aug 15 2023, 3:01 AM · Contributor Reviews (src)
minsoochoo0122_proton.me updated the diff for D41421: Update jemalloc to version 5.3.0.

Created separate revision for pthread_getname_np and _pthread_getname_np. This revision depends on D41461

Aug 15 2023, 2:41 AM · Contributor Reviews (src)
emaste updated subscribers of D41461: libc: make pthread_getname_np and _pthread_getname_np available.
Aug 15 2023, 2:40 AM · Contributor Reviews (src)
imp added a comment to D41461: libc: make pthread_getname_np and _pthread_getname_np available.

I wonder why these were not exported... Is there any indication as to why? Was it intentoinal or an oversight?

Aug 15 2023, 2:33 AM · Contributor Reviews (src)