On 05/15/2018 05:21 PM, Matthew Wilcox wrote: > On Tue, May 15, 2018 at 02:45:12PM -0400, Waiman Long wrote: >> On 05/15/2018 02:02 PM, Matthew Wilcox wrote: >>> On Tue, May 15, 2018 at 07:58:05PM +0200, Peter Zijlstra wrote: >>>> On Tue, May 15, 2018 at 01:38:04PM -0400, Waiman Long wrote: >>>>> +/* >>>>> + * Owner value to indicate the rwsem's owner is not currently known. >>>>> + */ >>>>> +#define RWSEM_OWNER_UNKNOWN ((struct task_struct *)-1) >>>> It might be nice to comment that this works and relies on having that >>>> ANON_OWNER bit set. >>> I'd rather change the definition to be ((struct task_struct *)2) >>> otherwise this is both reader-owned and anonymously-owned which doesn't >>> make much sense. >> Thinking about it a bit more. I can actually just use one special bit >> (bit 0) to designate an unknown owner. So for a reader-owned lock, it is >> just owner == 1 as the owners are unknown for a reader owned lock. For a >> lock owned by an unknown writer, it is (owner & 1) && (owner != 1). That >> will justify the use of -1L and save bit 1 for future extension. > To quote from your patch: > > - * In essence, the owner field now has the following 3 states: > + * In essence, the owner field now has the following 4 states: > * 1) 0 > * - lock is free or the owner hasn't set the field yet > * 2) RWSEM_READER_OWNED > * - lock is currently or previously owned by readers (lock is free > * or not set by owner yet) > - * 3) Other non-zero value > - * - a writer owns the lock > + * 3) RWSEM_ANONYMOUSLY_OWNED > + * - lock is owned by an anonymous writer, so spinning on the lock > + * owner should be disabled. > + * 4) Other non-zero value > + * - a writer owns the lock and other writers can spin on the lock owner. > > I'd leave these as 0, 1, 2, other. It's not really worth messing with > testing bits. > > Actually, if you change them to all be values -- s/NULL/RWSEM_NO_OWNER/ > > then you could define them as: > > RWSEM_READER_OWNED 0 > RWSEM_ANON_OWNED 1 > RWSEM_NO_OWNER 2 > > and rwsem_should_spin() is just sem->owner > 1. I would like to have owner equal to NULL if it is not locked. If it is locked, the owner can be used to get information about the owner. So I am not sure your scheme will work. Cheers, Longman
