public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] xfs: sparse fixes
@ 2014-09-24 23:37 Dave Chinner
  2014-09-24 23:37 ` [PATCH 1/4] xfs: fix use of agi_newino in finobt lookup Dave Chinner
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Dave Chinner @ 2014-09-24 23:37 UTC (permalink / raw)
  To: xfs

Hi folks,

In gathering up the outstanding sparse warning fixes, these are
the ones that went unnoticed or didn't have patches for them. The
first patch fixes an actual bug, though one with almost no impact
on performance and no impact on correctness. The others are just
cleanups to remove warnings from correctly functioning code.

Cheers,

Dave.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/4] xfs: fix use of agi_newino in finobt lookup
  2014-09-24 23:37 [PATCH 0/4] xfs: sparse fixes Dave Chinner
@ 2014-09-24 23:37 ` Dave Chinner
  2014-09-26  9:47   ` Christoph Hellwig
  2014-09-24 23:37 ` [PATCH 2/4] xfs: xfs_qm_dquot_isolate needs locking annotations for sparse Dave Chinner
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Dave Chinner @ 2014-09-24 23:37 UTC (permalink / raw)
  To: xfs

Sparse warns that we are passing the big-endian valueo f agi_newino
to the initial btree lookup function when trying to find a new
inode. This is wrong - we need to pass the host order value, not the
disk order value. This will adversely affect the next inode
allocated, but given that the free inode btree is usually much
smaller than the allocated inode btree it is much less likely to be
a performance issue if we start the search in the wrong place.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/libxfs/xfs_ialloc.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index d213a2e..23dcb72 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -1076,8 +1076,8 @@ xfs_dialloc_ag_finobt_newino(
 	int i;
 
 	if (agi->agi_newino != cpu_to_be32(NULLAGINO)) {
-		error = xfs_inobt_lookup(cur, agi->agi_newino, XFS_LOOKUP_EQ,
-					 &i);
+		error = xfs_inobt_lookup(cur, be32_to_cpu(agi->agi_newino),
+					 XFS_LOOKUP_EQ, &i);
 		if (error)
 			return error;
 		if (i == 1) {
@@ -1085,7 +1085,6 @@ xfs_dialloc_ag_finobt_newino(
 			if (error)
 				return error;
 			XFS_WANT_CORRUPTED_RETURN(i == 1);
-
 			return 0;
 		}
 	}
-- 
2.0.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/4] xfs: xfs_qm_dquot_isolate needs locking annotations for sparse
  2014-09-24 23:37 [PATCH 0/4] xfs: sparse fixes Dave Chinner
  2014-09-24 23:37 ` [PATCH 1/4] xfs: fix use of agi_newino in finobt lookup Dave Chinner
@ 2014-09-24 23:37 ` Dave Chinner
  2014-09-26  9:48   ` Christoph Hellwig
  2014-09-24 23:37 ` [PATCH 3/4] xfs: xfs_kset should be static Dave Chinner
  2014-09-24 23:37 ` [PATCH 4/4] xfs: annotate user variables passed as void Dave Chinner
  3 siblings, 1 reply; 9+ messages in thread
From: Dave Chinner @ 2014-09-24 23:37 UTC (permalink / raw)
  To: xfs

From: Dave Chinner <dchinner@redhat.com>

To remove noise from the build.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_qm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 1023210..d68f230 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -434,6 +434,7 @@ xfs_qm_dquot_isolate(
 	struct list_head	*item,
 	spinlock_t		*lru_lock,
 	void			*arg)
+		__releases(lru_lock) __acquires(lru_lock)
 {
 	struct xfs_dquot	*dqp = container_of(item,
 						struct xfs_dquot, q_lru);
-- 
2.0.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/4] xfs: xfs_kset should be static
  2014-09-24 23:37 [PATCH 0/4] xfs: sparse fixes Dave Chinner
  2014-09-24 23:37 ` [PATCH 1/4] xfs: fix use of agi_newino in finobt lookup Dave Chinner
  2014-09-24 23:37 ` [PATCH 2/4] xfs: xfs_qm_dquot_isolate needs locking annotations for sparse Dave Chinner
@ 2014-09-24 23:37 ` Dave Chinner
  2014-09-26  9:48   ` Christoph Hellwig
  2014-09-24 23:37 ` [PATCH 4/4] xfs: annotate user variables passed as void Dave Chinner
  3 siblings, 1 reply; 9+ messages in thread
From: Dave Chinner @ 2014-09-24 23:37 UTC (permalink / raw)
  To: xfs

From: Dave Chinner <dchinner@redhat.com>

As it is accessed through the struct xfs_mount and can be set up
entirely from fs/xfs/xfs_super.c

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_mount.c | 3 ---
 fs/xfs/xfs_super.c | 3 ++-
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index fbf0384..d36bdbc 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -61,8 +61,6 @@ static DEFINE_MUTEX(xfs_uuid_table_mutex);
 static int xfs_uuid_table_size;
 static uuid_t *xfs_uuid_table;
 
-extern struct kset *xfs_kset;
-
 /*
  * See if the UUID is unique among mounted XFS filesystems.
  * Mount fails if UUID is nil or a FS with the same UUID is already mounted.
@@ -729,7 +727,6 @@ xfs_mountfs(
 
 	xfs_set_maxicount(mp);
 
-	mp->m_kobj.kobject.kset = xfs_kset;
 	error = xfs_sysfs_init(&mp->m_kobj, &xfs_mp_ktype, NULL, mp->m_fsname);
 	if (error)
 		goto out;
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index dcd4b93..9f622fe 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -63,7 +63,7 @@ static const struct super_operations xfs_super_operations;
 static kmem_zone_t *xfs_ioend_zone;
 mempool_t *xfs_ioend_pool;
 
-struct kset *xfs_kset;			/* top-level xfs sysfs dir */
+static struct kset *xfs_kset;		/* top-level xfs sysfs dir */
 #ifdef DEBUG
 static struct xfs_kobj xfs_dbg_kobj;	/* global debug sysfs attrs */
 #endif
@@ -1411,6 +1411,7 @@ xfs_fs_fill_super(
 	atomic_set(&mp->m_active_trans, 0);
 	INIT_DELAYED_WORK(&mp->m_reclaim_work, xfs_reclaim_worker);
 	INIT_DELAYED_WORK(&mp->m_eofblocks_work, xfs_eofblocks_worker);
+	mp->m_kobj.kobject.kset = xfs_kset;
 
 	mp->m_super = sb;
 	sb->s_fs_info = mp;
-- 
2.0.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/4] xfs: annotate user variables passed as void
  2014-09-24 23:37 [PATCH 0/4] xfs: sparse fixes Dave Chinner
                   ` (2 preceding siblings ...)
  2014-09-24 23:37 ` [PATCH 3/4] xfs: xfs_kset should be static Dave Chinner
@ 2014-09-24 23:37 ` Dave Chinner
  2014-09-26  9:59   ` Christoph Hellwig
  3 siblings, 1 reply; 9+ messages in thread
From: Dave Chinner @ 2014-09-24 23:37 UTC (permalink / raw)
  To: xfs

From: Dave Chinner <dchinner@redhat.com>

Some argument callbacks can contain user buffers, and sparse warns
about passing them as void pointers. Cast appropriately to remove
the sparse warnings.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_ioctl.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 3799695..7a6b406 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1349,7 +1349,7 @@ xfs_ioc_setxflags(
 STATIC int
 xfs_getbmap_format(void **ap, struct getbmapx *bmv, int *full)
 {
-	struct getbmap __user	*base = *ap;
+	struct getbmap __user	*base = (struct getbmap __user *)*ap;
 
 	/* copy only getbmap portion (not getbmapx) */
 	if (copy_to_user(base, bmv, sizeof(struct getbmap)))
@@ -1380,7 +1380,7 @@ xfs_ioc_getbmap(
 		bmx.bmv_iflags |= BMV_IF_NO_DMAPI_READ;
 
 	error = xfs_getbmap(ip, &bmx, xfs_getbmap_format,
-			    (struct getbmap *)arg+1);
+			    (__force struct getbmap *)arg+1);
 	if (error)
 		return error;
 
@@ -1393,7 +1393,7 @@ xfs_ioc_getbmap(
 STATIC int
 xfs_getbmapx_format(void **ap, struct getbmapx *bmv, int *full)
 {
-	struct getbmapx __user	*base = *ap;
+	struct getbmapx __user	*base = (struct getbmapx __user *)*ap;
 
 	if (copy_to_user(base, bmv, sizeof(struct getbmapx)))
 		return -EFAULT;
@@ -1420,7 +1420,7 @@ xfs_ioc_getbmapx(
 		return -EINVAL;
 
 	error = xfs_getbmap(ip, &bmx, xfs_getbmapx_format,
-			    (struct getbmapx *)arg+1);
+			    (__force struct getbmapx *)arg+1);
 	if (error)
 		return error;
 
-- 
2.0.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] xfs: fix use of agi_newino in finobt lookup
  2014-09-24 23:37 ` [PATCH 1/4] xfs: fix use of agi_newino in finobt lookup Dave Chinner
@ 2014-09-26  9:47   ` Christoph Hellwig
  0 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2014-09-26  9:47 UTC (permalink / raw)
  To: Dave Chinner; +Cc: xfs

On Thu, Sep 25, 2014 at 09:37:40AM +1000, Dave Chinner wrote:
> Sparse warns that we are passing the big-endian valueo f agi_newino
> to the initial btree lookup function when trying to find a new
> inode. This is wrong - we need to pass the host order value, not the
> disk order value. This will adversely affect the next inode
> allocated, but given that the free inode btree is usually much
> smaller than the allocated inode btree it is much less likely to be
> a performance issue if we start the search in the wrong place.

Looks good.  I'm rather surprised this wasn't noticed earlier.

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] 9+ messages in thread

* Re: [PATCH 2/4] xfs: xfs_qm_dquot_isolate needs locking annotations for sparse
  2014-09-24 23:37 ` [PATCH 2/4] xfs: xfs_qm_dquot_isolate needs locking annotations for sparse Dave Chinner
@ 2014-09-26  9:48   ` Christoph Hellwig
  0 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2014-09-26  9:48 UTC (permalink / raw)
  To: Dave Chinner; +Cc: xfs

On Thu, Sep 25, 2014 at 09:37:41AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> To remove noise from the build.
> 
> 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] 9+ messages in thread

* Re: [PATCH 3/4] xfs: xfs_kset should be static
  2014-09-24 23:37 ` [PATCH 3/4] xfs: xfs_kset should be static Dave Chinner
@ 2014-09-26  9:48   ` Christoph Hellwig
  0 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2014-09-26  9:48 UTC (permalink / raw)
  To: Dave Chinner; +Cc: xfs

On Thu, Sep 25, 2014 at 09:37:42AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> As it is accessed through the struct xfs_mount and can be set up
> entirely from fs/xfs/xfs_super.c
> 
> 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] 9+ messages in thread

* Re: [PATCH 4/4] xfs: annotate user variables passed as void
  2014-09-24 23:37 ` [PATCH 4/4] xfs: annotate user variables passed as void Dave Chinner
@ 2014-09-26  9:59   ` Christoph Hellwig
  0 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2014-09-26  9:59 UTC (permalink / raw)
  To: Dave Chinner; +Cc: xfs

On Thu, Sep 25, 2014 at 09:37:43AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Some argument callbacks can contain user buffers, and sparse warns
> about passing them as void pointers. Cast appropriately to remove
> the sparse warnings.

Looks okay.  I don't really like the games we're playing there, but this
isn't the time to fix them up..

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] 9+ messages in thread

end of thread, other threads:[~2014-09-26  9:59 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-24 23:37 [PATCH 0/4] xfs: sparse fixes Dave Chinner
2014-09-24 23:37 ` [PATCH 1/4] xfs: fix use of agi_newino in finobt lookup Dave Chinner
2014-09-26  9:47   ` Christoph Hellwig
2014-09-24 23:37 ` [PATCH 2/4] xfs: xfs_qm_dquot_isolate needs locking annotations for sparse Dave Chinner
2014-09-26  9:48   ` Christoph Hellwig
2014-09-24 23:37 ` [PATCH 3/4] xfs: xfs_kset should be static Dave Chinner
2014-09-26  9:48   ` Christoph Hellwig
2014-09-24 23:37 ` [PATCH 4/4] xfs: annotate user variables passed as void Dave Chinner
2014-09-26  9:59   ` Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox