From: Omar Sandoval <osandov@osandov.com>
To: David Sterba <dsterba@suse.cz>
Cc: Al Viro <viro@ZenIV.linux.org.uk>,
linux-fsdevel@vger.kernel.org, linux-btrfs@vger.kernel.org,
ceph-devel@vger.kernel.org, linux-cifs@vger.kernel.org,
osd-dev@open-osd.org, linux-ext4@vger.kernel.org,
linux-f2fs-devel@lists.sourceforge.net,
fuse-devel@lists.sourceforge.net, cluster-devel@redhat.com,
jfs-discussion@lists.sourceforge.net, HPDD-discuss@ml01.01.org,
linux-nfs@vger.kernel.org, linux-nilfs@vger.kernel.org,
ocfs2-devel@oss.oracle.com, reiserfs-devel@vger.kernel.org,
v9fs-developer@lists.sourceforge.net, xfs@oss.sgi.com,
linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 1/5] new helper: iov_iter_rw()
Date: Tue, 17 Mar 2015 03:18:35 -0700 [thread overview]
Message-ID: <20150317101835.GA31649@mew> (raw)
In-Reply-To: <20150317093151.GS20767@twin.jikos.cz>
On Tue, Mar 17, 2015 at 10:31:51AM +0100, David Sterba wrote:
> On Mon, Mar 16, 2015 at 05:36:05PM +0000, Al Viro wrote:
> > On Mon, Mar 16, 2015 at 04:33:49AM -0700, Omar Sandoval wrote:
> > > Get either READ or WRITE out of iter->type.
> >
> > Umm...
> >
> > > + * Get one of READ or WRITE out of iter->type without any other flags OR'd in
> > > + * with it.
> > > + */
> > > +static inline int iov_iter_rw(const struct iov_iter *i)
> > > +{
> > > + return i->type & RW_MASK;
> > > +}
> >
> > TBH, I would turn that into a macro. Reason: indirect includes.
>
> Agreed, but the proposed define is rather cryptic and I was not able to
> understand the meaning on the first glance.
>
> > #define iov_iter_rw(i) ((0 ? (struct iov_iter *)0 : (i))->type & RW_MASK)
>
> This worked for me, does not compile with anything else than
> 'struct iov_iter*' as i:
>
> #define iov_iter_rw(i) ({ \
> struct iov_iter __iter = *(i); \
> (i)->type & RW_MASK; \
> })
>
> The assignment is optimized out.
[-cc individual fs maintainers to avoid all of these email bounces,
should've looked a bit closer at that get_maintainer.pl output...]
I agree that this is a bit more readable, but it evaluates i twice.
That's an easy fix, just do __iter.type instead of (i)->type, but
there's still the possibility of someone passing in something called
__iter as i, and the fix for that tends to be "add more underscores". At
the very least, Al's macro could probably use a comment explaining
what's going on there, though.
--
Omar
WARNING: multiple messages have this Message-ID (diff)
From: Omar Sandoval <osandov@osandov.com>
To: David Sterba <dsterba@suse.cz>
Cc: linux-cifs@vger.kernel.org, jfs-discussion@lists.sourceforge.net,
linux-nilfs@vger.kernel.org, reiserfs-devel@vger.kernel.org,
fuse-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org,
xfs@oss.sgi.com, linux-nfs@vger.kernel.org,
linux-f2fs-devel@lists.sourceforge.net, cluster-devel@redhat.com,
HPDD-discuss@ml01.01.org, ocfs2-devel@oss.oracle.com,
Al Viro <viro@ZenIV.linux.org.uk>,
linux-fsdevel@vger.kernel.org,
v9fs-developer@lists.sourceforge.net, ceph-devel@vger.kernel.org,
linux-ext4@vger.kernel.org, linux-btrfs@vger.kernel.org,
osd-dev@open-osd.org
Subject: Re: [RFC PATCH 1/5] new helper: iov_iter_rw()
Date: Tue, 17 Mar 2015 03:18:35 -0700 [thread overview]
Message-ID: <20150317101835.GA31649@mew> (raw)
In-Reply-To: <20150317093151.GS20767@twin.jikos.cz>
On Tue, Mar 17, 2015 at 10:31:51AM +0100, David Sterba wrote:
> On Mon, Mar 16, 2015 at 05:36:05PM +0000, Al Viro wrote:
> > On Mon, Mar 16, 2015 at 04:33:49AM -0700, Omar Sandoval wrote:
> > > Get either READ or WRITE out of iter->type.
> >
> > Umm...
> >
> > > + * Get one of READ or WRITE out of iter->type without any other flags OR'd in
> > > + * with it.
> > > + */
> > > +static inline int iov_iter_rw(const struct iov_iter *i)
> > > +{
> > > + return i->type & RW_MASK;
> > > +}
> >
> > TBH, I would turn that into a macro. Reason: indirect includes.
>
> Agreed, but the proposed define is rather cryptic and I was not able to
> understand the meaning on the first glance.
>
> > #define iov_iter_rw(i) ((0 ? (struct iov_iter *)0 : (i))->type & RW_MASK)
>
> This worked for me, does not compile with anything else than
> 'struct iov_iter*' as i:
>
> #define iov_iter_rw(i) ({ \
> struct iov_iter __iter = *(i); \
> (i)->type & RW_MASK; \
> })
>
> The assignment is optimized out.
[-cc individual fs maintainers to avoid all of these email bounces,
should've looked a bit closer at that get_maintainer.pl output...]
I agree that this is a bit more readable, but it evaluates i twice.
That's an easy fix, just do __iter.type instead of (i)->type, but
there's still the possibility of someone passing in something called
__iter as i, and the fix for that tends to be "add more underscores". At
the very least, Al's macro could probably use a comment explaining
what's going on there, though.
--
Omar
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
WARNING: multiple messages have this Message-ID (diff)
From: Omar Sandoval <osandov@osandov.com>
To: David Sterba <dsterba@suse.cz>
Cc: Al Viro <viro@ZenIV.linux.org.uk>,
linux-fsdevel@vger.kernel.org, linux-btrfs@vger.kernel.org,
ceph-devel@vger.kernel.org, linux-cifs@vger.kernel.org,
osd-dev@open-osd.org, linux-ext4@vger.kernel.org,
linux-f2fs-devel@lists.sourceforge.net,
fuse-devel@lists.sourceforge.net, cluster-devel@redhat.com,
jfs-discussion@lists.sourceforge.net, HPDD-discuss@ml01.01.org,
linux-nfs@vger.kernel.org, linux-nilfs@vger.kernel.org,
ocfs2-devel@oss.oracle.com, reiserfs-devel@vger.kernel.org,
v9fs-developer@lists.sourceforge.net, xfs@oss.sgi.com,
linux-kernel@vger.kernel.org
Subject: [Ocfs2-devel] [RFC PATCH 1/5] new helper: iov_iter_rw()
Date: Tue, 17 Mar 2015 03:18:35 -0700 [thread overview]
Message-ID: <20150317101835.GA31649@mew> (raw)
In-Reply-To: <20150317093151.GS20767@twin.jikos.cz>
On Tue, Mar 17, 2015 at 10:31:51AM +0100, David Sterba wrote:
> On Mon, Mar 16, 2015 at 05:36:05PM +0000, Al Viro wrote:
> > On Mon, Mar 16, 2015 at 04:33:49AM -0700, Omar Sandoval wrote:
> > > Get either READ or WRITE out of iter->type.
> >
> > Umm...
> >
> > > + * Get one of READ or WRITE out of iter->type without any other flags OR'd in
> > > + * with it.
> > > + */
> > > +static inline int iov_iter_rw(const struct iov_iter *i)
> > > +{
> > > + return i->type & RW_MASK;
> > > +}
> >
> > TBH, I would turn that into a macro. Reason: indirect includes.
>
> Agreed, but the proposed define is rather cryptic and I was not able to
> understand the meaning on the first glance.
>
> > #define iov_iter_rw(i) ((0 ? (struct iov_iter *)0 : (i))->type & RW_MASK)
>
> This worked for me, does not compile with anything else than
> 'struct iov_iter*' as i:
>
> #define iov_iter_rw(i) ({ \
> struct iov_iter __iter = *(i); \
> (i)->type & RW_MASK; \
> })
>
> The assignment is optimized out.
[-cc individual fs maintainers to avoid all of these email bounces,
should've looked a bit closer at that get_maintainer.pl output...]
I agree that this is a bit more readable, but it evaluates i twice.
That's an easy fix, just do __iter.type instead of (i)->type, but
there's still the possibility of someone passing in something called
__iter as i, and the fix for that tends to be "add more underscores". At
the very least, Al's macro could probably use a comment explaining
what's going on there, though.
--
Omar
WARNING: multiple messages have this Message-ID (diff)
From: Omar Sandoval <osandov@osandov.com>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [RFC PATCH 1/5] new helper: iov_iter_rw()
Date: Tue, 17 Mar 2015 03:18:35 -0700 [thread overview]
Message-ID: <20150317101835.GA31649@mew> (raw)
In-Reply-To: <20150317093151.GS20767@twin.jikos.cz>
On Tue, Mar 17, 2015 at 10:31:51AM +0100, David Sterba wrote:
> On Mon, Mar 16, 2015 at 05:36:05PM +0000, Al Viro wrote:
> > On Mon, Mar 16, 2015 at 04:33:49AM -0700, Omar Sandoval wrote:
> > > Get either READ or WRITE out of iter->type.
> >
> > Umm...
> >
> > > + * Get one of READ or WRITE out of iter->type without any other flags OR'd in
> > > + * with it.
> > > + */
> > > +static inline int iov_iter_rw(const struct iov_iter *i)
> > > +{
> > > + return i->type & RW_MASK;
> > > +}
> >
> > TBH, I would turn that into a macro. Reason: indirect includes.
>
> Agreed, but the proposed define is rather cryptic and I was not able to
> understand the meaning on the first glance.
>
> > #define iov_iter_rw(i) ((0 ? (struct iov_iter *)0 : (i))->type & RW_MASK)
>
> This worked for me, does not compile with anything else than
> 'struct iov_iter*' as i:
>
> #define iov_iter_rw(i) ({ \
> struct iov_iter __iter = *(i); \
> (i)->type & RW_MASK; \
> })
>
> The assignment is optimized out.
[-cc individual fs maintainers to avoid all of these email bounces,
should've looked a bit closer at that get_maintainer.pl output...]
I agree that this is a bit more readable, but it evaluates i twice.
That's an easy fix, just do __iter.type instead of (i)->type, but
there's still the possibility of someone passing in something called
__iter as i, and the fix for that tends to be "add more underscores". At
the very least, Al's macro could probably use a comment explaining
what's going on there, though.
--
Omar
next prev parent reply other threads:[~2015-03-17 10:18 UTC|newest]
Thread overview: 73+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-16 11:33 [RFC PATCH 0/5] Remove rw parameter from direct_IO() Omar Sandoval
2015-03-16 11:33 ` [Cluster-devel] " Omar Sandoval
2015-03-16 11:33 ` [Ocfs2-devel] " Omar Sandoval
2015-03-16 11:33 ` Omar Sandoval
2015-03-16 11:33 ` Omar Sandoval
2015-03-16 11:33 ` Omar Sandoval
2015-03-16 11:33 ` [RFC PATCH 1/5] new helper: iov_iter_rw() Omar Sandoval
2015-03-16 11:33 ` [Cluster-devel] " Omar Sandoval
2015-03-16 11:33 ` [Ocfs2-devel] " Omar Sandoval
2015-03-16 11:33 ` Omar Sandoval
2015-03-16 11:33 ` Omar Sandoval
2015-03-16 11:33 ` Omar Sandoval
2015-03-16 17:36 ` Al Viro
2015-03-16 17:36 ` [Cluster-devel] " Al Viro
2015-03-16 17:36 ` [Ocfs2-devel] " Al Viro
2015-03-16 17:36 ` Al Viro
2015-03-16 17:36 ` Al Viro
2015-03-16 17:36 ` Al Viro
2015-03-17 1:20 ` [RFC PATCH v2 " Omar Sandoval
2015-03-17 9:31 ` [RFC PATCH " David Sterba
2015-03-17 9:31 ` [Cluster-devel] " David Sterba
2015-03-17 9:31 ` [Ocfs2-devel] " David Sterba
2015-03-17 9:31 ` David Sterba
2015-03-17 9:31 ` David Sterba
2015-03-17 10:18 ` Omar Sandoval [this message]
2015-03-17 10:18 ` [Cluster-devel] " Omar Sandoval
2015-03-17 10:18 ` [Ocfs2-devel] " Omar Sandoval
2015-03-17 10:18 ` Omar Sandoval
2015-03-17 18:19 ` Al Viro
2015-03-17 18:19 ` Al Viro
2015-03-17 18:19 ` [Cluster-devel] " Al Viro
2015-03-17 18:19 ` [Ocfs2-devel] " Al Viro
2015-03-17 18:19 ` Al Viro
2015-03-17 18:19 ` Al Viro
2015-03-17 18:19 ` Al Viro
2015-03-17 21:04 ` [RFC PATCH v3 " Omar Sandoval
2015-03-18 13:42 ` David Sterba
2015-03-16 11:33 ` [RFC PATCH 2/5] Remove rw from {,__,do_}blockdev_direct_IO() Omar Sandoval
2015-03-16 11:33 ` [Cluster-devel] [RFC PATCH 2/5] Remove rw from {, __, do_}blockdev_direct_IO() Omar Sandoval
2015-03-16 11:33 ` [Ocfs2-devel] " Omar Sandoval
2015-03-16 11:33 ` [RFC PATCH 2/5] Remove rw from {,__,do_}blockdev_direct_IO() Omar Sandoval
2015-03-16 11:33 ` Omar Sandoval
2015-03-16 11:33 ` Omar Sandoval
2015-03-16 11:33 ` [RFC PATCH 3/5] Remove rw from dax_{do_,}io() Omar Sandoval
2015-03-16 11:33 ` [Cluster-devel] " Omar Sandoval
2015-03-16 11:33 ` [Ocfs2-devel] " Omar Sandoval
2015-03-16 11:33 ` Omar Sandoval
2015-03-16 11:33 ` Omar Sandoval
2015-03-16 11:33 ` Omar Sandoval
2015-03-16 11:33 ` [RFC PATCH 4/5] direct_IO: use iov_iter_rw() instead of rw everywhere Omar Sandoval
2015-03-16 11:33 ` [Cluster-devel] " Omar Sandoval
2015-03-16 11:33 ` [Ocfs2-devel] " Omar Sandoval
2015-03-16 11:33 ` Omar Sandoval
2015-03-16 11:33 ` Omar Sandoval
2015-03-16 11:33 ` Omar Sandoval
2015-03-16 11:33 ` [RFC PATCH 5/5] direct_IO: remove rw from a_ops->direct_IO() Omar Sandoval
2015-03-16 11:33 ` [Cluster-devel] " Omar Sandoval
2015-03-16 11:33 ` [Ocfs2-devel] " Omar Sandoval
2015-03-16 11:33 ` Omar Sandoval
2015-03-16 11:33 ` Omar Sandoval
2015-03-16 11:33 ` Omar Sandoval
2015-03-16 18:15 ` [RFC PATCH 0/5] Remove rw parameter from direct_IO() Al Viro
2015-03-16 18:15 ` [Cluster-devel] " Al Viro
2015-03-16 18:15 ` [Ocfs2-devel] " Al Viro
2015-03-16 18:15 ` Al Viro
2015-03-16 18:15 ` Al Viro
2015-03-16 18:15 ` Al Viro
2015-04-05 16:27 ` Al Viro
2015-04-05 16:27 ` [Cluster-devel] " Al Viro
2015-04-05 16:27 ` [Ocfs2-devel] " Al Viro
2015-04-05 16:27 ` Al Viro
2015-04-05 16:27 ` Al Viro
2015-04-05 16:27 ` Al Viro
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=20150317101835.GA31649@mew \
--to=osandov@osandov.com \
--cc=HPDD-discuss@ml01.01.org \
--cc=ceph-devel@vger.kernel.org \
--cc=cluster-devel@redhat.com \
--cc=dsterba@suse.cz \
--cc=fuse-devel@lists.sourceforge.net \
--cc=jfs-discussion@lists.sourceforge.net \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-cifs@vger.kernel.org \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=linux-nilfs@vger.kernel.org \
--cc=ocfs2-devel@oss.oracle.com \
--cc=osd-dev@open-osd.org \
--cc=reiserfs-devel@vger.kernel.org \
--cc=v9fs-developer@lists.sourceforge.net \
--cc=viro@ZenIV.linux.org.uk \
--cc=xfs@oss.sgi.com \
/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.