* [PATCH 0/2]: xfs - fixes found with sparse checking @ 2011-12-21 0:07 Dave Chinner 2011-12-21 0:07 ` [PATCH 1/2] xfs: clean up minor sparse warnings Dave Chinner 2011-12-21 0:07 ` [PATCH 2/2] xfs: fix endian conversion issue in discard code Dave Chinner 0 siblings, 2 replies; 11+ messages in thread From: Dave Chinner @ 2011-12-21 0:07 UTC (permalink / raw) To: xfs There are a few minor issues. The first patch are all really just cleanups that don't change functionality. The second patch fixes a real bug that we need to consider backporting to stable releases. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] xfs: clean up minor sparse warnings 2011-12-21 0:07 [PATCH 0/2]: xfs - fixes found with sparse checking Dave Chinner @ 2011-12-21 0:07 ` Dave Chinner 2011-12-21 16:13 ` Christoph Hellwig 2011-12-21 0:07 ` [PATCH 2/2] xfs: fix endian conversion issue in discard code Dave Chinner 1 sibling, 1 reply; 11+ messages in thread From: Dave Chinner @ 2011-12-21 0:07 UTC (permalink / raw) To: xfs From: Dave Chinner <dchinner@redhat.com> Signed-off-by: Dave Chinner <dchinner@redhat.com> --- fs/xfs/xfs_dir2_block.c | 1 + fs/xfs/xfs_ioctl.c | 10 ++++------ fs/xfs/xfs_ioctl32.c | 2 +- fs/xfs/xfs_iops.c | 13 ++++++++----- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/fs/xfs/xfs_dir2_block.c b/fs/xfs/xfs_dir2_block.c index 9245e02..d3b63ae 100644 --- a/fs/xfs/xfs_dir2_block.c +++ b/fs/xfs/xfs_dir2_block.c @@ -29,6 +29,7 @@ #include "xfs_dinode.h" #include "xfs_inode.h" #include "xfs_inode_item.h" +#include "xfs_dir2.h" #include "xfs_dir2_format.h" #include "xfs_dir2_priv.h" #include "xfs_error.h" diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c index d99a905..2f3f56a 100644 --- a/fs/xfs/xfs_ioctl.c +++ b/fs/xfs/xfs_ioctl.c @@ -230,24 +230,22 @@ xfs_open_by_handle( /* Put open permission in namei format. */ permflag = hreq->oflags; - if ((permflag+1) & O_ACCMODE) - permflag++; if (permflag & O_TRUNC) - permflag |= 2; + permflag |= O_RDWR; if ((!(permflag & O_APPEND) || (permflag & O_TRUNC)) && - (permflag & FMODE_WRITE) && IS_APPEND(inode)) { + (OPEN_FMODE(permflag) & FMODE_WRITE) && IS_APPEND(inode)) { error = -XFS_ERROR(EPERM); goto out_dput; } - if ((permflag & FMODE_WRITE) && IS_IMMUTABLE(inode)) { + if ((OPEN_FMODE(permflag) & FMODE_WRITE) && IS_IMMUTABLE(inode)) { error = -XFS_ERROR(EACCES); goto out_dput; } /* Can't write directories. */ - if (S_ISDIR(inode->i_mode) && (permflag & FMODE_WRITE)) { + if (S_ISDIR(inode->i_mode) && (OPEN_FMODE(permflag) & FMODE_WRITE)) { error = -XFS_ERROR(EISDIR); goto out_dput; } diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c index 54e623b..ce332af 100644 --- a/fs/xfs/xfs_ioctl32.c +++ b/fs/xfs/xfs_ioctl32.c @@ -293,7 +293,7 @@ xfs_compat_ioc_bulkstat( int res; error = xfs_bulkstat_one_compat(mp, inlast, bulkreq.ubuffer, - sizeof(compat_xfs_bstat_t), 0, &res); + sizeof(compat_xfs_bstat_t), NULL, &res); } else if (cmd == XFS_IOC_FSBULKSTAT_32) { error = xfs_bulkstat(mp, &inlast, &count, xfs_bulkstat_one_compat, sizeof(compat_xfs_bstat_t), diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c index 23ce927..a5a9772 100644 --- a/fs/xfs/xfs_iops.c +++ b/fs/xfs/xfs_iops.c @@ -103,12 +103,15 @@ xfs_mark_inode_dirty( } -int xfs_initxattrs(struct inode *inode, const struct xattr *xattr_array, - void *fs_info) +static int +xfs_initxattrs( + struct inode *inode, + const struct xattr *xattr_array, + void *fs_info) { - const struct xattr *xattr; - struct xfs_inode *ip = XFS_I(inode); - int error = 0; + const struct xattr *xattr; + struct xfs_inode *ip = XFS_I(inode); + int error = 0; for (xattr = xattr_array; xattr->name != NULL; xattr++) { error = xfs_attr_set(ip, xattr->name, xattr->value, -- 1.7.5.4 _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] xfs: clean up minor sparse warnings 2011-12-21 0:07 ` [PATCH 1/2] xfs: clean up minor sparse warnings Dave Chinner @ 2011-12-21 16:13 ` Christoph Hellwig 2011-12-21 23:59 ` Dave Chinner 0 siblings, 1 reply; 11+ messages in thread From: Christoph Hellwig @ 2011-12-21 16:13 UTC (permalink / raw) To: Dave Chinner; +Cc: xfs > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c > index d99a905..2f3f56a 100644 > --- a/fs/xfs/xfs_ioctl.c > +++ b/fs/xfs/xfs_ioctl.c > @@ -230,24 +230,22 @@ xfs_open_by_handle( > > /* Put open permission in namei format. */ > permflag = hreq->oflags; > - if ((permflag+1) & O_ACCMODE) > - permflag++; > if (permflag & O_TRUNC) > - permflag |= 2; > + permflag |= O_RDWR; > > if ((!(permflag & O_APPEND) || (permflag & O_TRUNC)) && > - (permflag & FMODE_WRITE) && IS_APPEND(inode)) { > + (OPEN_FMODE(permflag) & FMODE_WRITE) && IS_APPEND(inode)) { > error = -XFS_ERROR(EPERM); > goto out_dput; > } > > - if ((permflag & FMODE_WRITE) && IS_IMMUTABLE(inode)) { > + if ((OPEN_FMODE(permflag) & FMODE_WRITE) && IS_IMMUTABLE(inode)) { > error = -XFS_ERROR(EACCES); > goto out_dput; > } > > /* Can't write directories. */ > - if (S_ISDIR(inode->i_mode) && (permflag & FMODE_WRITE)) { > + if (S_ISDIR(inode->i_mode) && (OPEN_FMODE(permflag) & FMODE_WRITE)) { > error = -XFS_ERROR(EISDIR); > goto out_dput; > } I think this one is complicated enough that is deserves a separate patch and a better description. For the other small bits: Reviewed-by: Christoph Hellwig <hch@lst.de> _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] xfs: clean up minor sparse warnings 2011-12-21 16:13 ` Christoph Hellwig @ 2011-12-21 23:59 ` Dave Chinner 2011-12-22 18:15 ` Ben Myers 2012-02-01 18:33 ` Ben Myers 0 siblings, 2 replies; 11+ messages in thread From: Dave Chinner @ 2011-12-21 23:59 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs On Wed, Dec 21, 2011 at 11:13:50AM -0500, Christoph Hellwig wrote: > > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c > > index d99a905..2f3f56a 100644 > > --- a/fs/xfs/xfs_ioctl.c > > +++ b/fs/xfs/xfs_ioctl.c > > @@ -230,24 +230,22 @@ xfs_open_by_handle( > > > > /* Put open permission in namei format. */ > > permflag = hreq->oflags; > > - if ((permflag+1) & O_ACCMODE) > > - permflag++; > > if (permflag & O_TRUNC) > > - permflag |= 2; > > + permflag |= O_RDWR; > > > > if ((!(permflag & O_APPEND) || (permflag & O_TRUNC)) && > > - (permflag & FMODE_WRITE) && IS_APPEND(inode)) { > > + (OPEN_FMODE(permflag) & FMODE_WRITE) && IS_APPEND(inode)) { > > error = -XFS_ERROR(EPERM); > > goto out_dput; > > } > > > > - if ((permflag & FMODE_WRITE) && IS_IMMUTABLE(inode)) { > > + if ((OPEN_FMODE(permflag) & FMODE_WRITE) && IS_IMMUTABLE(inode)) { > > error = -XFS_ERROR(EACCES); > > goto out_dput; > > } > > > > /* Can't write directories. */ > > - if (S_ISDIR(inode->i_mode) && (permflag & FMODE_WRITE)) { > > + if (S_ISDIR(inode->i_mode) && (OPEN_FMODE(permflag) & FMODE_WRITE)) { > > error = -XFS_ERROR(EISDIR); > > goto out_dput; > > } > > I think this one is complicated enough that is deserves a separate > patch and a better description. Ok, I've also just found a problem with it(*) so I'll separate it and resend when I've fixed it. Cheers, Dave. (*) did you know we actually have a test (079) that verifies this logic? I didn't until a little while ago.... -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] xfs: clean up minor sparse warnings 2011-12-21 23:59 ` Dave Chinner @ 2011-12-22 18:15 ` Ben Myers 2012-02-01 18:33 ` Ben Myers 1 sibling, 0 replies; 11+ messages in thread From: Ben Myers @ 2011-12-22 18:15 UTC (permalink / raw) To: Dave Chinner; +Cc: Christoph Hellwig, xfs Hey Dave, On Thu, Dec 22, 2011 at 10:59:04AM +1100, Dave Chinner wrote: > On Wed, Dec 21, 2011 at 11:13:50AM -0500, Christoph Hellwig wrote: > > > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c > > > index d99a905..2f3f56a 100644 > > > --- a/fs/xfs/xfs_ioctl.c > > > +++ b/fs/xfs/xfs_ioctl.c > > > @@ -230,24 +230,22 @@ xfs_open_by_handle( > > > > > > /* Put open permission in namei format. */ > > > permflag = hreq->oflags; > > > - if ((permflag+1) & O_ACCMODE) > > > - permflag++; > > > if (permflag & O_TRUNC) > > > - permflag |= 2; > > > + permflag |= O_RDWR; > > > > > > if ((!(permflag & O_APPEND) || (permflag & O_TRUNC)) && > > > - (permflag & FMODE_WRITE) && IS_APPEND(inode)) { > > > + (OPEN_FMODE(permflag) & FMODE_WRITE) && IS_APPEND(inode)) { > > > error = -XFS_ERROR(EPERM); > > > goto out_dput; > > > } > > > > > > - if ((permflag & FMODE_WRITE) && IS_IMMUTABLE(inode)) { > > > + if ((OPEN_FMODE(permflag) & FMODE_WRITE) && IS_IMMUTABLE(inode)) { > > > error = -XFS_ERROR(EACCES); > > > goto out_dput; > > > } > > > > > > /* Can't write directories. */ > > > - if (S_ISDIR(inode->i_mode) && (permflag & FMODE_WRITE)) { > > > + if (S_ISDIR(inode->i_mode) && (OPEN_FMODE(permflag) & FMODE_WRITE)) { > > > error = -XFS_ERROR(EISDIR); > > > goto out_dput; > > > } > > > > I think this one is complicated enough that is deserves a separate > > patch and a better description. > > Ok, I've also just found a problem with it(*) so I'll separate it > and resend when I've fixed it. Reviewed-by: Ben Myers <bpm@sgi.com> D'oh, I just reviewed this and didn't find the problem. Looking forward to your next rev to see what it is. -Ben _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] xfs: clean up minor sparse warnings 2011-12-21 23:59 ` Dave Chinner 2011-12-22 18:15 ` Ben Myers @ 2012-02-01 18:33 ` Ben Myers 1 sibling, 0 replies; 11+ messages in thread From: Ben Myers @ 2012-02-01 18:33 UTC (permalink / raw) To: Dave Chinner; +Cc: Christoph Hellwig, xfs Hey Dave, On Thu, Dec 22, 2011 at 10:59:04AM +1100, Dave Chinner wrote: > On Wed, Dec 21, 2011 at 11:13:50AM -0500, Christoph Hellwig wrote: > > > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c > > > index d99a905..2f3f56a 100644 > > > --- a/fs/xfs/xfs_ioctl.c > > > +++ b/fs/xfs/xfs_ioctl.c > > > @@ -230,24 +230,22 @@ xfs_open_by_handle( > > > > > > /* Put open permission in namei format. */ > > > permflag = hreq->oflags; > > > - if ((permflag+1) & O_ACCMODE) > > > - permflag++; > > > if (permflag & O_TRUNC) > > > - permflag |= 2; > > > + permflag |= O_RDWR; > > > > > > if ((!(permflag & O_APPEND) || (permflag & O_TRUNC)) && > > > - (permflag & FMODE_WRITE) && IS_APPEND(inode)) { > > > + (OPEN_FMODE(permflag) & FMODE_WRITE) && IS_APPEND(inode)) { > > > error = -XFS_ERROR(EPERM); > > > goto out_dput; > > > } > > > > > > - if ((permflag & FMODE_WRITE) && IS_IMMUTABLE(inode)) { > > > + if ((OPEN_FMODE(permflag) & FMODE_WRITE) && IS_IMMUTABLE(inode)) { > > > error = -XFS_ERROR(EACCES); > > > goto out_dput; > > > } > > > > > > /* Can't write directories. */ > > > - if (S_ISDIR(inode->i_mode) && (permflag & FMODE_WRITE)) { > > > + if (S_ISDIR(inode->i_mode) && (OPEN_FMODE(permflag) & FMODE_WRITE)) { > > > error = -XFS_ERROR(EISDIR); > > > goto out_dput; > > > } > > > > I think this one is complicated enough that is deserves a separate > > patch and a better description. > > Ok, I've also just found a problem with it(*) so I'll separate it > and resend when I've fixed it. Did you resend this? Thanks, Ben _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] xfs: fix endian conversion issue in discard code 2011-12-21 0:07 [PATCH 0/2]: xfs - fixes found with sparse checking Dave Chinner 2011-12-21 0:07 ` [PATCH 1/2] xfs: clean up minor sparse warnings Dave Chinner @ 2011-12-21 0:07 ` Dave Chinner 2011-12-21 4:36 ` Ben Myers 2011-12-21 16:11 ` Christoph Hellwig 1 sibling, 2 replies; 11+ messages in thread From: Dave Chinner @ 2011-12-21 0:07 UTC (permalink / raw) To: xfs From: Dave Chinner <dchinner@redhat.com> When finding the longest extent in an AG, we read the value directly out of the AGF buffer without endian conversion. This will give an incorrect length, resulting in FITRIM operations potentially not trimming everything that it should. Signed-off-by: Dave Chinner <dchinner@redhat.com> --- fs/xfs/xfs_discard.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c index 8a24f0c..286a051 100644 --- a/fs/xfs/xfs_discard.c +++ b/fs/xfs/xfs_discard.c @@ -68,7 +68,7 @@ xfs_trim_extents( * Look up the longest btree in the AGF and start with it. */ error = xfs_alloc_lookup_le(cur, 0, - XFS_BUF_TO_AGF(agbp)->agf_longest, &i); + be32_to_cpu(XFS_BUF_TO_AGF(agbp)->agf_longest), &i); if (error) goto out_del_cursor; @@ -84,7 +84,7 @@ xfs_trim_extents( if (error) goto out_del_cursor; XFS_WANT_CORRUPTED_GOTO(i == 1, out_del_cursor); - ASSERT(flen <= XFS_BUF_TO_AGF(agbp)->agf_longest); + ASSERT(flen <= be32_to_cpu(XFS_BUF_TO_AGF(agbp)->agf_longest)); /* * Too small? Give up. -- 1.7.5.4 _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] xfs: fix endian conversion issue in discard code 2011-12-21 0:07 ` [PATCH 2/2] xfs: fix endian conversion issue in discard code Dave Chinner @ 2011-12-21 4:36 ` Ben Myers 2012-01-06 15:12 ` Christoph Hellwig 2011-12-21 16:11 ` Christoph Hellwig 1 sibling, 1 reply; 11+ messages in thread From: Ben Myers @ 2011-12-21 4:36 UTC (permalink / raw) To: Dave Chinner; +Cc: xfs On Wed, Dec 21, 2011 at 11:07:42AM +1100, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > When finding the longest extent in an AG, we read the value directly > out of the AGF buffer without endian conversion. This will give an > incorrect length, resulting in FITRIM operations potentially not > trimming everything that it should. Looks good to me. > Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Ben Myers <bpm@sgi.com> _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] xfs: fix endian conversion issue in discard code 2011-12-21 4:36 ` Ben Myers @ 2012-01-06 15:12 ` Christoph Hellwig 2012-01-06 15:38 ` Ben Myers 0 siblings, 1 reply; 11+ messages in thread From: Christoph Hellwig @ 2012-01-06 15:12 UTC (permalink / raw) To: Ben Myers; +Cc: xfs On Tue, Dec 20, 2011 at 10:36:38PM -0600, Ben Myers wrote: > On Wed, Dec 21, 2011 at 11:07:42AM +1100, Dave Chinner wrote: > > From: Dave Chinner <dchinner@redhat.com> > > > > When finding the longest extent in an AG, we read the value directly > > out of the AGF buffer without endian conversion. This will give an > > incorrect length, resulting in FITRIM operations potentially not > > trimming everything that it should. > > Looks good to me. > > > Signed-off-by: Dave Chinner <dchinner@redhat.com> > > Reviewed-by: Ben Myers <bpm@sgi.com> This is a pretty serious thing, can you send it to Linus as soon as he takes the current pull request? _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] xfs: fix endian conversion issue in discard code 2012-01-06 15:12 ` Christoph Hellwig @ 2012-01-06 15:38 ` Ben Myers 0 siblings, 0 replies; 11+ messages in thread From: Ben Myers @ 2012-01-06 15:38 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs On Fri, Jan 06, 2012 at 10:12:54AM -0500, Christoph Hellwig wrote: > On Tue, Dec 20, 2011 at 10:36:38PM -0600, Ben Myers wrote: > > On Wed, Dec 21, 2011 at 11:07:42AM +1100, Dave Chinner wrote: > > > From: Dave Chinner <dchinner@redhat.com> > > > > > > When finding the longest extent in an AG, we read the value directly > > > out of the AGF buffer without endian conversion. This will give an > > > incorrect length, resulting in FITRIM operations potentially not > > > trimming everything that it should. > > > > Looks good to me. > > > > > Signed-off-by: Dave Chinner <dchinner@redhat.com> > > > > Reviewed-by: Ben Myers <bpm@sgi.com> > > This is a pretty serious thing, can you send it to Linus as soon as > he takes the current pull request? yep. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] xfs: fix endian conversion issue in discard code 2011-12-21 0:07 ` [PATCH 2/2] xfs: fix endian conversion issue in discard code Dave Chinner 2011-12-21 4:36 ` Ben Myers @ 2011-12-21 16:11 ` Christoph Hellwig 1 sibling, 0 replies; 11+ messages in thread From: Christoph Hellwig @ 2011-12-21 16:11 UTC (permalink / raw) To: Dave Chinner; +Cc: xfs On Wed, Dec 21, 2011 at 11:07:42AM +1100, Dave Chinner wrote: > From: Dave Chinner <dchinner@redhat.com> > > When finding the longest extent in an AG, we read the value directly > out of the AGF buffer without endian conversion. This will give an > incorrect length, resulting in FITRIM operations potentially not > trimming everything that it should. > > Signed-off-by: Dave Chinner <dchinner@redhat.com> Looks good, Reviewed-by: Christoph Hellwig <hch@lst.de> _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-02-01 18:33 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-12-21 0:07 [PATCH 0/2]: xfs - fixes found with sparse checking Dave Chinner 2011-12-21 0:07 ` [PATCH 1/2] xfs: clean up minor sparse warnings Dave Chinner 2011-12-21 16:13 ` Christoph Hellwig 2011-12-21 23:59 ` Dave Chinner 2011-12-22 18:15 ` Ben Myers 2012-02-01 18:33 ` Ben Myers 2011-12-21 0:07 ` [PATCH 2/2] xfs: fix endian conversion issue in discard code Dave Chinner 2011-12-21 4:36 ` Ben Myers 2012-01-06 15:12 ` Christoph Hellwig 2012-01-06 15:38 ` Ben Myers 2011-12-21 16:11 ` Christoph Hellwig
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox