Re: [PATCH 21/26] tty: Convert tty_struct bitfield to bools

[Peter Hurley]

On 09/03/2014 01:56 PM, Greg Kroah-Hartman wrote:
> On Wed, Sep 03, 2014 at 11:10:45AM -0400, Peter Hurley wrote:
>> On 09/03/2014 08:19 AM, One Thousand Gnomes wrote:
>>>> Ahh. Thanks for the insight, Alan.
>>>>
>>>> But set_bit() et. al. will generate an incredible amount of churn;
>>>> what if I split the fields up to prevent false-sharing?
>>>
>>> Do you feel lucky ;-)
>>
>> Hahaha  :)
>>
>>> I'd rather set_bit and friends were used. They exist largely for this
>>> kind of reason and they also have atomic test/set methods which may in
>>> the longer term be very useful.
>>>
>>> Yes it is churn can't argue with that.
>>
>> Yuck. There should be a better way. IXANY mode is suddenly going to
>> have a ton of unnecessary bus locks on x86.
> 
> True, but at least it will be correct, which I'm guessing today it isn't
> :(
> 
>> Note the ctrl_status field is a byte as well, which can't be RMW'ed by
>> the bit-locked primitives, and definitely should not be aggregated with
>> any adjacent field.
> 
> Never trust what an ia64 compiler can, and will, do...

I could not get the ia64 compiler to merge adjacent reads or writes on
bools or chars, at all.

However, Alan's point applies anyway --
because the Alpha is not byte-addressable.

This means that smaller-than-int storage is _always_ read-modify-write.

So my plan is now to take this patch out of this series and submit
a new series on top of this one that will:
1) merge the ctrl_status field and packet field into an int size, which
   will guarantee atomicity to those fields on all arches when accessed
   holding the ->ctrl_lock.
2) make tty->hw_stopped an int. This way legacy drivers -- synclink,
   cyclades, nozomi, isdn4linux, et. al. -- will not need changes.
   UART drivers should use uport->hw_stopped (added in this series).
   New, non-UART drivers should use their own hw_stopped mechanism.
3) keep tty->stopped and tty->flow_stopped as bitfield members of the
   same bitfield, but size that bitfield to unsigned long [note 1 below].
   This should guarantee atomicity on all arches when accessed while
   holding the ->flow_lock, without requiring further code changes and
   without impacting other arches.

Regards,
Peter Hurley

[Note 1] gcc pre-4.7.2 corrupts data adjacent to a smaller-than-8-byte
bitfield on sparc64, ia64, ppc64 and alpha.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/