This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Linux: Implement per-thread file system attributes
On 6/28/19 12:55 PM, Florian Weimer wrote:
> * Carlos O'Donell:
>
>> The thread_perthreadfs_indirect uses the NULL attribute,
>> and not PTHREAD_PER_PROCESS_NP explicitly, and it's an
>> implementation detail that this happens to be the same
>> code path e.g. we copy the default global attr, then adjust
>> the scope flags. This might change in the future and it would
>> be nice to catch any breakage in a distinct test for this.
>>
>> The test would be a basic flag test looking to validate the
>> stickyness in a simpler test.
>>
>> The simpler test should have the parent threads permute over
>> [null, per-process, per-thread], and the child iterate over
>> [null, per-process, per-thread], and validate the expected
>> results.
>
> Is it sufficient to verify that the thread attribute is set correctly,
> or do you want a fully functional test?
It is sufficient to verify that the thread attribute is set correctly.
> If the latter, I will split the indirect flag into direct, indirect with
> default attribute, and indirect with non-default attribute because I
> would have to replicate a lot of the other test functionality to verify
> per-thread separation or the lack thereof.
That sounds reasonable to me if you want to keep the test as a single test.
>>> +/* Return true if the thread has per-thread file system
>>> + attributes. */
>>> +static bool
>>> +perthread_flag (pthread_t thr)
>>> +{
>>> + pthread_attr_t attr;
>>> + int ret = pthread_getattr_np (thr, &attr);
>>> + if (ret != 0)
>>> + {
>>> + errno = ret;
>>> + FAIL_EXIT1 ("pthread_getattr_np: %m");
>>> + }
>>> + int flag = -1;
>>> + pthread_attr_getperthreadfs_np (&attr, &flag);
>>> + if (flag != 0)
>>> + TEST_COMPARE (flag, 1);
>>
>> Magic number.
>>
>> Please use some intermediate define to call this out.
>>
>> #define CHECK_PTHREAD_PER_THREAD_FS 1
>>
>> ... and use in place of 1.
>>
>> This way it's grep-able when looking for references to
>> the enumerated value.
>
> Right, I realized that as well. I've now got:
>
> /* Return true if the thread has per-thread file system
> attributes. */
> static bool
> perthread_flag (pthread_t thr)
> {
> pthread_attr_t attr;
> int ret = pthread_getattr_np (thr, &attr);
> if (ret != 0)
> {
> errno = ret;
> FAIL_EXIT1 ("pthread_getattr_np: %m");
> }
> int flag = -1;
> pthread_attr_getperthreadfs_np (&attr, &flag);
> if (flag != PTHREAD_PER_THREAD_NP)
> TEST_COMPARE (flag, PTHREAD_PER_PROCESS_NP);
I don't follow why you can't call this unconditionally?
> xpthread_attr_destroy (&attr);
> return flag == PTHREAD_PER_THREAD_NP;
OK.
> }
>
> Thanks,
> Florian
>
--
Cheers,
Carlos.