From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCHv3 2/2] spinlock: fair read-write locks Date: Wed, 03 Feb 2016 05:14:43 -0700 Message-ID: <56B1FD4302000078000CDFBB@prv-mh.provo.novell.com> References: <1454326273-23588-1-git-send-email-david.vrabel@citrix.com> <1454326273-23588-3-git-send-email-david.vrabel@citrix.com> <56B1F28902000078000CDF09@prv-mh.provo.novell.com> <56B1EB37.90407@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1aQwKg-0005uK-OX for xen-devel@lists.xenproject.org; Wed, 03 Feb 2016 12:14:50 +0000 In-Reply-To: <56B1EB37.90407@citrix.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: David Vrabel , Jennifer Herbert Cc: xen-devel@lists.xenproject.org, Ian Campbell List-Id: xen-devel@lists.xenproject.org >>> On 03.02.16 at 12:57, wrote: > On 03/02/16 11:28, Jan Beulich wrote: >>>>> On 01.02.16 at 12:31, wrote: >>> +void queue_write_lock_slowpath(rwlock_t *lock) >>> +{ >>> + u32 cnts; >>> + >>> + /* Put the writer into the wait queue. */ >>> + spin_lock(&lock->lock); >>> + >>> + /* Try to acquire the lock directly if no reader is present. */ >>> + if ( !atomic_read(&lock->cnts) && >>> + (atomic_cmpxchg(&lock->cnts, 0, _QW_LOCKED) == 0) ) >>> + goto unlock; >>> + >>> + /* >>> + * Set the waiting flag to notify readers that a writer is pending, >>> + * or wait for a previous writer to go away. >>> + */ >>> + for (;;) >> >> Since everything else here has been nicely converted to Xen style, >> strictly speaking these should be >> >> for ( ; ; ) >> >> but of course this is no reason to block the patch. Since however, >> as said in reply to patch 1, ... > > TBH, I really think you're pointlessly nit-picking here. This change > would make zero impact on readability. Well, the style I've pointed out is the one used in well formed code (and in fact I have to remind myself of this each time I omit one or more of the three expressions). But as said, I'm not demanding a re-submission just because of this (and if I remember to do so, I may well fix it up while committing). >>> --- a/xen/include/xen/rwlock.h >>> +++ b/xen/include/xen/rwlock.h >>> @@ -3,6 +3,188 @@ >>> >>> #include >> >> ... this should go away if possible, it would be nice for the cosmetic >> thing above to also be fixed up at once. > > The rwlock structure now includes a spinlock, so this #include is > required here. Ah, okay, patch 2 indeed makes this necessary. Both patches Acked-by: Jan Beulich then. Jan