Page MenuHomeFreeBSD

pthread_attr_get_np(3): Proper attributes object lifecycle
ClosedPublic

Authored by olce on Jan 5 2024, 2:18 PM.
Tags
None
Referenced Files
Unknown Object (File)
Oct 10 2024, 10:10 PM
Unknown Object (File)
Oct 2 2024, 10:26 PM
Unknown Object (File)
Sep 30 2024, 4:43 AM
Unknown Object (File)
Sep 26 2024, 2:35 PM
Unknown Object (File)
Sep 24 2024, 5:49 PM
Unknown Object (File)
Sep 23 2024, 2:22 AM
Unknown Object (File)
Sep 22 2024, 2:19 AM
Unknown Object (File)
Sep 21 2024, 9:49 AM

Details

Summary

While here, rephrase unclear passages, add references and fix the
example's style.

MFC after: 2 weeks

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 55260
Build 52149: arc lint + arc unit

Event Timeline

olce requested review of this revision.Jan 5 2024, 2:18 PM
share/man/man3/pthread_attr_get_np.3
57–59

Wrt my comment in D43329 we should be explicit about object lifecycle requirements if pthread_attr_get_np fails

share/man/man3/pthread_attr_get_np.3
50

This is not true. Values can be updated, and the returned structure reflects the current settings.

51
99

I do not think that we guarantee anything if the dst arg is not properly initialized.

olce planned changes to this revision.Jan 5 2024, 9:31 PM
olce marked an inline comment as done.
olce added inline comments.
share/man/man3/pthread_attr_get_np.3
50

You're right, and I have been knowing it first hand since I've analyzed substantially this area and changed the priority-related attributes (not published yet). I was mostly concentrating on getting the initial changes right before coming to that part. But OK, let's do it while here.

99

In fact, we guarantee that a not explicitly initialized, but implicitly initialized to NULL, attributes object will cause EINVAL. Problem is, there is no implicit initialization for local variables. I'll fix the text.

olce marked 3 inline comments as done.

Fix the description of which data is returned. Add the guarantee that the passed object is left unmodified in case the function reports failure (depends on D43329). Mention ENOMEM as a possible return value.

And fix the EINVAL description.

share/man/man3/pthread_attr_get_np.3
44–45

New sentence new line

46

Stack top address is then still current, because you simply cannot change it after the thread start.

69

I do not understand this sentence.

81
82

and IMO this is too much to promise

olce marked 4 inline comments as done.

Address comments.

share/man/man3/pthread_attr_get_np.3
46

It's not current, since the value computed and used internally is never reflected back into the attributes object. We would need to modify cpu_set_upcall() to change that fact, and it's not in my plans. The fact that the stack top can't change after the thread start, besides being obvious, is completely independent.

69

The sentence inside parentheses? It is there to clarify that there is no obligation to provide a pointer to an object that has just been initialized by pthread_attr_init(), the latter may have been used for other purposes in the meantime (e.g., such as specifying attributes to use for some threads, before asking later for the current values).

82

I don't think so. This is already guaranteed by the limited changes of D43329 and arguably should be as a matter of good interface design (it falls under POLA according to me). I would agree with you if it had to come with a huge performance hit or overly complicated code, but that isn't the case here.

olce marked an inline comment as done.Jan 9 2024, 1:36 PM
share/man/man3/pthread_attr_get_np.3
69

So why not say directly that calls to pthread_attr_get_np() with the same pattr object do not require re-initialization?

pauamma_gundo.com added inline comments.
share/man/man3/pthread_attr_get_np.3
70

Or clarify what you mean by "regularly".

olce marked 2 inline comments as done.

Rework the part on the lifecycle pointed to by dst.

share/man/man3/pthread_attr_get_np.3
69

I think this is not general enough to convey what I would like to. My initial formulation doesn't seem to be much better. Also, I would like to avoid repeating what belongs to pthread_attr(3) (but is not even there).

I've updated the revision with a new one. What do you think?

70

(This has been rewritten anew.)

kib added inline comments.
share/man/man3/pthread_attr_get_np.3
82

It is too cumbersome to maintain, and does not give much benefits to users that typically alloc/get/dealloc.

This revision is now accepted and ready to land.Jan 10 2024, 4:52 PM
olce marked an inline comment as done.Jan 10 2024, 5:03 PM
olce added inline comments.
share/man/man3/pthread_attr_get_np.3
82

pthread_attr_get_np()'s code is not the simplest, but I don't find it very cumbersome either. And the previous significant revision of this function is dated... 2010. I don't think we'll have to change it at all in the next 10 years, except as a consequence of changes I may make in the course of the "priorities" project or as a follow-up. So...

This revision was automatically updated to reflect the committed changes.
olce marked an inline comment as done.

@pauamma_gundo.com Ah, I forgot to tag you as a reviewer in the commit. I'm sorry.