diff mbox

Disable single thread optimization for open_memstream

Message ID 59662DA4.1000702@arm.com
State New, archived
Headers

Commit Message

Szabolcs Nagy July 12, 2017, 2:09 p.m. UTC 
  Single thread optimization is valid if at thread creation time the
optimization can be disabled.  This is in principle true for all
stream objects that user code can access (and thus needs locking),
using the same internal list as fflush(0) uses.  However in glibc
open_memstream is not on that list (BZ 21735) so the optimization
has to be disabled.

2017-07-12  Szabolcs Nagy  <szabolcs.nagy@arm.com>

	* libio/memstream.c (__open_memstream): Set _IO_FLAGS2_NEED_LOCK.
	* libio/wmemstream.c (open_wmemstream): Likewise.

Comments

Florian Weimer July 12, 2017, 2:10 p.m. UTC | #1 
On 07/12/2017 04:09 PM, Szabolcs Nagy wrote:
> Single thread optimization is valid if at thread creation time the
> optimization can be disabled.  This is in principle true for all
> stream objects that user code can access (and thus needs locking),
> using the same internal list as fflush(0) uses.  However in glibc
> open_memstream is not on that list (BZ 21735) so the optimization
> has to be disabled.
> 
> 2017-07-12  Szabolcs Nagy  <szabolcs.nagy@arm.com>
> 
> 	* libio/memstream.c (__open_memstream): Set _IO_FLAGS2_NEED_LOCK.
> 	* libio/wmemstream.c (open_wmemstream): Likewise.

What about the test case I posted?

Florian
Adhemerval Zanella July 12, 2017, 2:14 p.m. UTC | #2 
On 12/07/2017 11:10, Florian Weimer wrote:
> On 07/12/2017 04:09 PM, Szabolcs Nagy wrote:
>> Single thread optimization is valid if at thread creation time the
>> optimization can be disabled.  This is in principle true for all
>> stream objects that user code can access (and thus needs locking),
>> using the same internal list as fflush(0) uses.  However in glibc
>> open_memstream is not on that list (BZ 21735) so the optimization
>> has to be disabled.
>>
>> 2017-07-12  Szabolcs Nagy  <szabolcs.nagy@arm.com>
>>
>> 	* libio/memstream.c (__open_memstream): Set _IO_FLAGS2_NEED_LOCK.
>> 	* libio/wmemstream.c (open_wmemstream): Likewise.
> 
> What about the test case I posted?
> 
> Florian
> 

I am about to update my BZ#21735 fix based on Florian's comment, as for
this workaround it should touch only memstream implementation.
Carlos O'Donell July 12, 2017, 2:15 p.m. UTC | #3 
On 07/12/2017 10:09 AM, Szabolcs Nagy wrote:
> Single thread optimization is valid if at thread creation time the
> optimization can be disabled.  This is in principle true for all
> stream objects that user code can access (and thus needs locking),
> using the same internal list as fflush(0) uses.  However in glibc
> open_memstream is not on that list (BZ 21735) so the optimization
> has to be disabled.
> 
> 2017-07-12  Szabolcs Nagy  <szabolcs.nagy@arm.com>
> 
> 	* libio/memstream.c (__open_memstream): Set _IO_FLAGS2_NEED_LOCK.
> 	* libio/wmemstream.c (open_wmemstream): Likewise.
> 

IIUC we can avoid this patch if we fix BZ #21735?
Szabolcs Nagy July 12, 2017, 2:20 p.m. UTC | #4 
On 12/07/17 15:15, Carlos O'Donell wrote:
> On 07/12/2017 10:09 AM, Szabolcs Nagy wrote:
>> Single thread optimization is valid if at thread creation time the
>> optimization can be disabled.  This is in principle true for all
>> stream objects that user code can access (and thus needs locking),
>> using the same internal list as fflush(0) uses.  However in glibc
>> open_memstream is not on that list (BZ 21735) so the optimization
>> has to be disabled.
>>
>> 2017-07-12  Szabolcs Nagy  <szabolcs.nagy@arm.com>
>>
>> 	* libio/memstream.c (__open_memstream): Set _IO_FLAGS2_NEED_LOCK.
>> 	* libio/wmemstream.c (open_wmemstream): Likewise.
>>
> 
> IIUC we can avoid this patch if we fix BZ #21735?
> 

yes, this is an alternative workaround that's less intrusive.
Szabolcs Nagy July 12, 2017, 2:22 p.m. UTC | #5 
On 12/07/17 15:10, Florian Weimer wrote:
> On 07/12/2017 04:09 PM, Szabolcs Nagy wrote:
>> Single thread optimization is valid if at thread creation time the
>> optimization can be disabled.  This is in principle true for all
>> stream objects that user code can access (and thus needs locking),
>> using the same internal list as fflush(0) uses.  However in glibc
>> open_memstream is not on that list (BZ 21735) so the optimization
>> has to be disabled.
>>
>> 2017-07-12  Szabolcs Nagy  <szabolcs.nagy@arm.com>
>>
>> 	* libio/memstream.c (__open_memstream): Set _IO_FLAGS2_NEED_LOCK.
>> 	* libio/wmemstream.c (open_wmemstream): Likewise.
> 
> What about the test case I posted?
> 

it passes now, but i did not add it to glibc.. may be i should have.

but now i'll wait for Adhemerval updating his patch.
diff mbox

Patch

diff --git a/libio/memstream.c b/libio/memstream.c
index f83d4a5213..e391efd48a 100644
--- a/libio/memstream.c
+++ b/libio/memstream.c
@@ -96,6 +96,9 @@  __open_memstream (char **bufloc, _IO_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 (_IO_FILE *) &new_f->fp._sf._sbf;
 }
 libc_hidden_def (__open_memstream)
diff --git a/libio/wmemstream.c b/libio/wmemstream.c
index 5bc77f52ee..103a760bf5 100644
--- a/libio/wmemstream.c
+++ b/libio/wmemstream.c
@@ -98,6 +98,9 @@  open_wmemstream (wchar_t **bufloc, _IO_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 (_IO_FILE *) &new_f->fp._sf._sbf;
 }