Messages in this thread | | | Date | Tue, 10 May 2016 09:02:19 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH v2] locking/rwsem: Add reader-owned state to the owner field |
| |
On Mon, May 09, 2016 at 10:24:16PM -0400, Waiman Long wrote: > >>+static inline void rwsem_set_reader_owned(struct rw_semaphore *sem) > >>+{ > >>+ /* > >>+ * We check the owner value first to make sure that we will only > >>+ * do a write to the rwsem cacheline when it is really necessary > >>+ * to minimize cacheline contention. > >>+ */ > >>+ if (sem->owner != (struct task_struct *)RWSEM_READER_OWNED) > >>+ sem->owner = (struct task_struct *)RWSEM_READER_OWNED; > >How much if anything did this optimization matter? > > I hadn't run any performance test to verify the effective of this change. > For a reader-heavy rwsem, this change should be able to save quite a lot of > needless write to the rwsem cacheline.
Right; I was just wondering.
> >>+static inline bool rwsem_is_writer_owned(struct task_struct *owner) > >>+{ > >>+ return (unsigned long)owner> RWSEM_READER_OWNED; > >>+} > >Tad too clever that; what does GCC generate if you write the obvious: > > > > return owner&& owner != RWSEM_READER_OWNER; > > You are right. GCC is intelligent enough to make the necessary optimization. > I will revert it to this form which is more obvious.
Yay! thanks for checking. Sometimes GCC throws a wobbly and does something unexpected, but it tends to get these 'simple' things right.
| |