All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ross Zwisler <ross.zwisler@linux.intel.com>
To: Josh Triplett <josh@joshtriplett.org>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>,
	Matthew Wilcox <willy@infradead.org>,
	linux-kernel@vger.kernel.org, Dave Hansen <dave.hansen@intel.com>,
	linux-mm@kvack.org, Matthew Wilcox <mawilcox@microsoft.com>
Subject: Re: [PATCH 2/2] Introduce __cond_lock_err
Date: Thu, 21 Dec 2017 15:10:50 -0700	[thread overview]
Message-ID: <20171221221050.GD9087@linux.intel.com> (raw)
In-Reply-To: <20171221220015.GA14919@localhost>

On Thu, Dec 21, 2017 at 02:00:16PM -0800, Josh Triplett wrote:
> On Thu, Dec 21, 2017 at 02:48:10PM -0700, Ross Zwisler wrote:
> > On Tue, Dec 19, 2017 at 08:58:23AM -0800, Matthew Wilcox wrote:
> > > From: Matthew Wilcox <mawilcox@microsoft.com>
> > > 
> > > The __cond_lock macro expects the function to return 'true' if the lock
> > > was acquired and 'false' if it wasn't.  We have another common calling
> > > convention in the kernel, which is returning 0 on success and an errno
> > > on failure.  It's hard to use the existing __cond_lock macro for those
> > > kinds of functions, so introduce __cond_lock_err() and convert the
> > > two existing users.
> > 
> > This is much cleaner!  One quick issue below.
> > 
> > > Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
> > > ---
> > >  include/linux/compiler_types.h | 2 ++
> > >  include/linux/mm.h             | 9 ++-------
> > >  mm/memory.c                    | 9 ++-------
> > >  3 files changed, 6 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> > > index 6b79a9bba9a7..ff3c41c78efa 100644
> > > --- a/include/linux/compiler_types.h
> > > +++ b/include/linux/compiler_types.h
> > > @@ -16,6 +16,7 @@
> > >  # define __acquire(x)	__context__(x,1)
> > >  # define __release(x)	__context__(x,-1)
> > >  # define __cond_lock(x,c)	((c) ? ({ __acquire(x); 1; }) : 0)
> > > +# define __cond_lock_err(x,c)	((c) ? 1 : ({ __acquire(x); 0; }))
> > 					       ^
> > I think we actually want this to return c here ^
> 
> Then you want to use ((c) ?: ...), to avoid evaluating c twice.

Oh, yep, great catch.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Ross Zwisler <ross.zwisler@linux.intel.com>
To: Josh Triplett <josh@joshtriplett.org>
Cc: Ross Zwisler <ross.zwisler@linux.intel.com>,
	Matthew Wilcox <willy@infradead.org>,
	linux-kernel@vger.kernel.org, Dave Hansen <dave.hansen@intel.com>,
	linux-mm@kvack.org, Matthew Wilcox <mawilcox@microsoft.com>
Subject: Re: [PATCH 2/2] Introduce __cond_lock_err
Date: Thu, 21 Dec 2017 15:10:50 -0700	[thread overview]
Message-ID: <20171221221050.GD9087@linux.intel.com> (raw)
In-Reply-To: <20171221220015.GA14919@localhost>

On Thu, Dec 21, 2017 at 02:00:16PM -0800, Josh Triplett wrote:
> On Thu, Dec 21, 2017 at 02:48:10PM -0700, Ross Zwisler wrote:
> > On Tue, Dec 19, 2017 at 08:58:23AM -0800, Matthew Wilcox wrote:
> > > From: Matthew Wilcox <mawilcox@microsoft.com>
> > > 
> > > The __cond_lock macro expects the function to return 'true' if the lock
> > > was acquired and 'false' if it wasn't.  We have another common calling
> > > convention in the kernel, which is returning 0 on success and an errno
> > > on failure.  It's hard to use the existing __cond_lock macro for those
> > > kinds of functions, so introduce __cond_lock_err() and convert the
> > > two existing users.
> > 
> > This is much cleaner!  One quick issue below.
> > 
> > > Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
> > > ---
> > >  include/linux/compiler_types.h | 2 ++
> > >  include/linux/mm.h             | 9 ++-------
> > >  mm/memory.c                    | 9 ++-------
> > >  3 files changed, 6 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> > > index 6b79a9bba9a7..ff3c41c78efa 100644
> > > --- a/include/linux/compiler_types.h
> > > +++ b/include/linux/compiler_types.h
> > > @@ -16,6 +16,7 @@
> > >  # define __acquire(x)	__context__(x,1)
> > >  # define __release(x)	__context__(x,-1)
> > >  # define __cond_lock(x,c)	((c) ? ({ __acquire(x); 1; }) : 0)
> > > +# define __cond_lock_err(x,c)	((c) ? 1 : ({ __acquire(x); 0; }))
> > 					       ^
> > I think we actually want this to return c here ^
> 
> Then you want to use ((c) ?: ...), to avoid evaluating c twice.

Oh, yep, great catch.

  reply	other threads:[~2017-12-21 22:10 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-19 16:58 [PATCH 1/2] mm: Make follow_pte_pmd an inline Matthew Wilcox
2017-12-19 16:58 ` Matthew Wilcox
2017-12-19 16:58 ` [PATCH 2/2] Introduce __cond_lock_err Matthew Wilcox
2017-12-19 16:58   ` Matthew Wilcox
2017-12-21 21:48   ` Ross Zwisler
2017-12-21 21:48     ` Ross Zwisler
2017-12-21 22:00     ` Josh Triplett
2017-12-21 22:00       ` Josh Triplett
2017-12-21 22:10       ` Ross Zwisler [this message]
2017-12-21 22:10         ` Ross Zwisler
2017-12-22  1:10     ` Matthew Wilcox
2017-12-22  1:10       ` Matthew Wilcox
2017-12-22  4:21       ` Josh Triplett
2017-12-22  4:21         ` Josh Triplett
2017-12-22 12:31         ` Matthew Wilcox
2017-12-22 12:31           ` Matthew Wilcox
2017-12-22 13:36           ` Matthew Wilcox
2017-12-22 13:36             ` Matthew Wilcox
2017-12-23  9:39             ` Josh Triplett
2017-12-23  9:39               ` Josh Triplett
2017-12-23 13:06               ` Matthew Wilcox
2017-12-23 13:06                 ` Matthew Wilcox
2017-12-27 14:38                 ` Luc Van Oostenryck
2017-12-27 14:38                   ` Luc Van Oostenryck
2017-12-27 14:28           ` Luc Van Oostenryck
2017-12-27 14:28             ` Luc Van Oostenryck
2017-12-30  7:17             ` Matthew Wilcox
2017-12-30  7:17               ` Matthew Wilcox
2017-12-19 17:05 ` [PATCH 1/2] mm: Make follow_pte_pmd an inline Joe Perches
2017-12-19 17:05   ` Joe Perches
2017-12-19 17:12   ` Matthew Wilcox
2017-12-19 17:12     ` Matthew Wilcox
2017-12-21 21:29 ` Ross Zwisler
2017-12-21 21:29   ` Ross Zwisler
2017-12-22  1:07   ` Matthew Wilcox
2017-12-22  1:07     ` Matthew Wilcox

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171221221050.GD9087@linux.intel.com \
    --to=ross.zwisler@linux.intel.com \
    --cc=dave.hansen@intel.com \
    --cc=josh@joshtriplett.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mawilcox@microsoft.com \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.