public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] a few cleanups for 2.6.36
@ 2010-07-18 20:38 Christoph Hellwig
  2010-07-18 20:38 ` [PATCH 1/6] xfs: fix gcc 4.6 set but not read and unused statement warnings Christoph Hellwig
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Christoph Hellwig @ 2010-07-18 20:38 UTC (permalink / raw)
  To: xfs

A couple of random cleanups, including those hunks from Andi's gcc 4.6
warning fixes that can easily be applied.

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

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

* [PATCH 1/6] xfs: fix gcc 4.6 set but not read and unused statement warnings
  2010-07-18 20:38 [PATCH 0/6] a few cleanups for 2.6.36 Christoph Hellwig
@ 2010-07-18 20:38 ` Christoph Hellwig
  2010-07-20  7:34   ` Dave Chinner
  2010-07-18 20:38 ` [PATCH 2/6] xfs: clean up filestreams helpers Christoph Hellwig
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Christoph Hellwig @ 2010-07-18 20:38 UTC (permalink / raw)
  To: xfs; +Cc: Andi Kleen

[-- Attachment #1: xfs-gcc46-warnings --]
[-- Type: text/plain, Size: 8384 bytes --]

[hch: dropped a few hunks that need structural changes instead]

Signed-off-by: Andi Kleen <ak@linux.intel.com>

Index: xfs/fs/xfs/xfs_da_btree.c
===================================================================
--- xfs.orig/fs/xfs/xfs_da_btree.c	2010-07-03 10:53:08.000000000 +0200
+++ xfs/fs/xfs/xfs_da_btree.c	2010-07-18 07:00:15.917494680 +0200
@@ -576,16 +576,14 @@ xfs_da_node_add(xfs_da_state_t *state, x
 	xfs_da_intnode_t *node;
 	xfs_da_node_entry_t *btree;
 	int tmp;
-	xfs_mount_t *mp;
 
 	node = oldblk->bp->data;
-	mp = state->mp;
 	ASSERT(be16_to_cpu(node->hdr.info.magic) == XFS_DA_NODE_MAGIC);
 	ASSERT((oldblk->index >= 0) && (oldblk->index <= be16_to_cpu(node->hdr.count)));
 	ASSERT(newblk->blkno != 0);
 	if (state->args->whichfork == XFS_DATA_FORK)
-		ASSERT(newblk->blkno >= mp->m_dirleafblk &&
-		       newblk->blkno < mp->m_dirfreeblk);
+		ASSERT(newblk->blkno >= state->mp->m_dirleafblk &&
+		       newblk->blkno < state->mp->m_dirfreeblk);
 
 	/*
 	 * We may need to make some room before we insert the new node.
Index: xfs/fs/xfs/xfs_dir2_block.c
===================================================================
--- xfs.orig/fs/xfs/xfs_dir2_block.c	2010-07-03 10:53:08.000000000 +0200
+++ xfs/fs/xfs/xfs_dir2_block.c	2010-07-18 07:00:15.917494680 +0200
@@ -1071,10 +1071,10 @@ xfs_dir2_sf_to_block(
 	 */
 
 	buf_len = dp->i_df.if_bytes;
-	buf = kmem_alloc(dp->i_df.if_bytes, KM_SLEEP);
+	buf = kmem_alloc(buf_len, KM_SLEEP);
 
-	memcpy(buf, sfp, dp->i_df.if_bytes);
-	xfs_idata_realloc(dp, -dp->i_df.if_bytes, XFS_DATA_FORK);
+	memcpy(buf, sfp, buf_len);
+	xfs_idata_realloc(dp, -buf_len, XFS_DATA_FORK);
 	dp->i_d.di_size = 0;
 	xfs_trans_log_inode(tp, dp, XFS_ILOG_CORE);
 	/*
Index: xfs/fs/xfs/xfs_iget.c
===================================================================
--- xfs.orig/fs/xfs/xfs_iget.c	2010-07-03 10:53:08.000000000 +0200
+++ xfs/fs/xfs/xfs_iget.c	2010-07-18 07:00:15.925494680 +0200
@@ -261,7 +261,6 @@ xfs_iget_cache_miss(
 {
 	struct xfs_inode	*ip;
 	int			error;
-	unsigned long		first_index, mask;
 	xfs_agino_t		agino = XFS_INO_TO_AGINO(mp, ino);
 
 	ip = xfs_inode_alloc(mp, ino);
@@ -298,8 +297,6 @@ xfs_iget_cache_miss(
 			BUG();
 	}
 
-	mask = ~(((XFS_INODE_CLUSTER_SIZE(mp) >> mp->m_sb.sb_inodelog)) - 1);
-	first_index = agino & mask;
 	write_lock(&pag->pag_ici_lock);
 
 	/* insert the new inode */
Index: xfs/fs/xfs/xfs_inode.c
===================================================================
--- xfs.orig/fs/xfs/xfs_inode.c	2010-07-16 08:07:57.855494681 +0200
+++ xfs/fs/xfs/xfs_inode.c	2010-07-18 07:00:15.930494680 +0200
@@ -918,7 +918,6 @@ xfs_iread_extents(
 	int		error;
 	xfs_ifork_t	*ifp;
 	xfs_extnum_t	nextents;
-	size_t		size;
 
 	if (unlikely(XFS_IFORK_FORMAT(ip, whichfork) != XFS_DINODE_FMT_BTREE)) {
 		XFS_ERROR_REPORT("xfs_iread_extents", XFS_ERRLEVEL_LOW,
@@ -926,7 +925,6 @@ xfs_iread_extents(
 		return XFS_ERROR(EFSCORRUPTED);
 	}
 	nextents = XFS_IFORK_NEXTENTS(ip, whichfork);
-	size = nextents * sizeof(xfs_bmbt_rec_t);
 	ifp = XFS_IFORK_PTR(ip, whichfork);
 
 	/*
@@ -3501,13 +3499,11 @@ xfs_iext_remove_indirect(
 	xfs_extnum_t	ext_diff;	/* extents to remove in current list */
 	xfs_extnum_t	nex1;		/* number of extents before idx */
 	xfs_extnum_t	nex2;		/* extents after idx + count */
-	int		nlists;		/* entries in indirection array */
 	int		page_idx = idx;	/* index in target extent list */
 
 	ASSERT(ifp->if_flags & XFS_IFEXTIREC);
 	erp = xfs_iext_idx_to_irec(ifp,  &page_idx, &erp_idx, 0);
 	ASSERT(erp != NULL);
-	nlists = ifp->if_real_bytes / XFS_IEXT_BUFSZ;
 	nex1 = page_idx;
 	ext_cnt = count;
 	while (ext_cnt) {
Index: xfs/fs/xfs/xfs_inode_item.c
===================================================================
--- xfs.orig/fs/xfs/xfs_inode_item.c	2010-07-03 10:53:08.000000000 +0200
+++ xfs/fs/xfs/xfs_inode_item.c	2010-07-18 07:00:15.935494680 +0200
@@ -214,7 +214,6 @@ xfs_inode_item_format(
 	uint			nvecs;
 	size_t			data_bytes;
 	xfs_bmbt_rec_t		*ext_buffer;
-	int			nrecs;
 	xfs_mount_t		*mp;
 
 	vecp->i_addr = &iip->ili_format;
@@ -314,9 +313,8 @@ xfs_inode_item_format(
 			ASSERT(ip->i_df.if_u1.if_extents != NULL);
 			ASSERT(ip->i_d.di_nextents > 0);
 			ASSERT(iip->ili_extents_buf == NULL);
-			nrecs = ip->i_df.if_bytes /
-				(uint)sizeof(xfs_bmbt_rec_t);
-			ASSERT(nrecs > 0);
+			ASSERT((ip->i_df.if_bytes /
+				(uint)sizeof(xfs_bmbt_rec_t)) > 0);
 #ifdef XFS_NATIVE_HOST
 			if (nrecs == ip->i_d.di_nextents) {
 				/*
@@ -439,15 +437,15 @@ xfs_inode_item_format(
 		ASSERT(!(iip->ili_format.ilf_fields &
 			 (XFS_ILOG_ADATA | XFS_ILOG_ABROOT)));
 		if (iip->ili_format.ilf_fields & XFS_ILOG_AEXT) {
-			ASSERT(ip->i_afp->if_bytes > 0);
-			ASSERT(ip->i_afp->if_u1.if_extents != NULL);
-			ASSERT(ip->i_d.di_anextents > 0);
 #ifdef DEBUG
-			nrecs = ip->i_afp->if_bytes /
+			int nrecs = ip->i_afp->if_bytes /
 				(uint)sizeof(xfs_bmbt_rec_t);
-#endif
 			ASSERT(nrecs > 0);
 			ASSERT(nrecs == ip->i_d.di_anextents);
+			ASSERT(ip->i_afp->if_bytes > 0);
+			ASSERT(ip->i_afp->if_u1.if_extents != NULL);
+			ASSERT(ip->i_d.di_anextents > 0);
+#endif
 #ifdef XFS_NATIVE_HOST
 			/*
 			 * There are not delayed allocation extents
@@ -889,10 +887,8 @@ xfs_iflush_abort(
 	xfs_inode_t		*ip)
 {
 	xfs_inode_log_item_t	*iip = ip->i_itemp;
-	xfs_mount_t		*mp;
 
 	iip = ip->i_itemp;
-	mp = ip->i_mount;
 	if (iip) {
 		struct xfs_ail	*ailp = iip->ili_item.li_ailp;
 		if (iip->ili_item.li_flags & XFS_LI_IN_AIL) {
Index: xfs/fs/xfs/xfs_log.c
===================================================================
--- xfs.orig/fs/xfs/xfs_log.c	2010-07-16 08:03:14.000000000 +0200
+++ xfs/fs/xfs/xfs_log.c	2010-07-18 07:00:15.941494680 +0200
@@ -1042,7 +1042,6 @@ xlog_alloc_log(xfs_mount_t	*mp,
 	xlog_in_core_t		*iclog, *prev_iclog=NULL;
 	xfs_buf_t		*bp;
 	int			i;
-	int			iclogsize;
 	int			error = ENOMEM;
 	uint			log2_size = 0;
 
@@ -1122,7 +1121,6 @@ xlog_alloc_log(xfs_mount_t	*mp,
 	 * with different amounts of memory.  See the definition of
 	 * xlog_in_core_t in xfs_log_priv.h for details.
 	 */
-	iclogsize = log->l_iclog_size;
 	ASSERT(log->l_iclog_size >= 4096);
 	for (i=0; i < log->l_iclog_bufs; i++) {
 		*iclogp = kmem_zalloc(sizeof(xlog_in_core_t), KM_MAYFAIL);
Index: xfs/fs/xfs/xfs_alloc.c
===================================================================
--- xfs.orig/fs/xfs/xfs_alloc.c	2010-07-18 06:58:25.017494682 +0200
+++ xfs/fs/xfs/xfs_alloc.c	2010-07-18 07:00:15.949494680 +0200
@@ -683,8 +683,6 @@ xfs_alloc_ag_vextent_near(
 	xfs_agblock_t	ltbno;		/* start bno of left side entry */
 	xfs_agblock_t	ltbnoa;		/* aligned ... */
 	xfs_extlen_t	ltdiff;		/* difference to left side entry */
-	/*REFERENCED*/
-	xfs_agblock_t	ltend;		/* end bno of left side entry */
 	xfs_extlen_t	ltlen;		/* length of left side entry */
 	xfs_extlen_t	ltlena;		/* aligned ... */
 	xfs_agblock_t	ltnew;		/* useful start bno of left side */
@@ -809,8 +807,7 @@ xfs_alloc_ag_vextent_near(
 		if ((error = xfs_alloc_get_rec(cnt_cur, &ltbno, &ltlen, &i)))
 			goto error0;
 		XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
-		ltend = ltbno + ltlen;
-		ASSERT(ltend <= be32_to_cpu(XFS_BUF_TO_AGF(args->agbp)->agf_length));
+		ASSERT(ltbno + ltlen <= be32_to_cpu(XFS_BUF_TO_AGF(args->agbp)->agf_length));
 		args->len = blen;
 		if (!xfs_alloc_fix_minleft(args)) {
 			xfs_btree_del_cursor(cnt_cur, XFS_BTREE_NOERROR);
@@ -823,7 +820,7 @@ xfs_alloc_ag_vextent_near(
 		 */
 		args->agbno = bnew;
 		ASSERT(bnew >= ltbno);
-		ASSERT(bnew + blen <= ltend);
+		ASSERT(bnew + blen <= ltbno + ltlen);
 		/*
 		 * Set up a cursor for the by-bno tree.
 		 */
@@ -1152,7 +1149,6 @@ xfs_alloc_ag_vextent_near(
 	/*
 	 * Fix up the length and compute the useful address.
 	 */
-	ltend = ltbno + ltlen;
 	args->len = XFS_EXTLEN_MIN(ltlena, args->maxlen);
 	xfs_alloc_fix_len(args);
 	if (!xfs_alloc_fix_minleft(args)) {
@@ -1165,7 +1161,7 @@ xfs_alloc_ag_vextent_near(
 	(void)xfs_alloc_compute_diff(args->agbno, rlen, args->alignment, ltbno,
 		ltlen, &ltnew);
 	ASSERT(ltnew >= ltbno);
-	ASSERT(ltnew + rlen <= ltend);
+	ASSERT(ltnew + rlen <= ltbno + ltlen);
 	ASSERT(ltnew + rlen <= be32_to_cpu(XFS_BUF_TO_AGF(args->agbp)->agf_length));
 	args->agbno = ltnew;
 	if ((error = xfs_alloc_fixup_trees(cnt_cur, bno_cur_lt, ltbno, ltlen,

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

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

* [PATCH 2/6] xfs: clean up filestreams helpers
  2010-07-18 20:38 [PATCH 0/6] a few cleanups for 2.6.36 Christoph Hellwig
  2010-07-18 20:38 ` [PATCH 1/6] xfs: fix gcc 4.6 set but not read and unused statement warnings Christoph Hellwig
@ 2010-07-18 20:38 ` Christoph Hellwig
  2010-07-20  7:35   ` Dave Chinner
  2010-07-18 20:38 ` [PATCH 3/6] xfs: remove obsolete osyncisosync mount option Christoph Hellwig
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Christoph Hellwig @ 2010-07-18 20:38 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: xfs-cleanup-filestreams-getput --]
[-- Type: text/plain, Size: 7710 bytes --]

Move xfs_filestream_peek_ag, xxfs_filestream_get_ag and xfs_filestream_put_ag
from xfs_filestream.h to xfs_filestream.c where it's only callers are, and
remove the inline marker while we're at it to let the compiler decide on the
inlining.  Also don't return a value from xfs_filestream_put_ag because
we don't need it.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs/fs/xfs/xfs_filestream.c
===================================================================
--- xfs.orig/fs/xfs/xfs_filestream.c	2010-07-18 15:10:45.662494680 +0200
+++ xfs/fs/xfs/xfs_filestream.c	2010-07-18 15:11:26.526494680 +0200
@@ -123,6 +123,82 @@ typedef struct fstrm_item
 	xfs_inode_t	*pip;	/* Parent directory inode pointer. */
 } fstrm_item_t;
 
+/*
+ * Allocation group filestream associations are tracked with per-ag atomic
+ * counters.  These counters allow _xfs_filestream_pick_ag() to tell whether a
+ * particular AG already has active filestreams associated with it. The mount
+ * point's m_peraglock is used to protect these counters from per-ag array
+ * re-allocation during a growfs operation.  When xfs_growfs_data_private() is
+ * about to reallocate the array, it calls xfs_filestream_flush() with the
+ * m_peraglock held in write mode.
+ *
+ * Since xfs_mru_cache_flush() guarantees that all the free functions for all
+ * the cache elements have finished executing before it returns, it's safe for
+ * the free functions to use the atomic counters without m_peraglock protection.
+ * This allows the implementation of xfs_fstrm_free_func() to be agnostic about
+ * whether it was called with the m_peraglock held in read mode, write mode or
+ * not held at all.  The race condition this addresses is the following:
+ *
+ *  - The work queue scheduler fires and pulls a filestream directory cache
+ *    element off the LRU end of the cache for deletion, then gets pre-empted.
+ *  - A growfs operation grabs the m_peraglock in write mode, flushes all the
+ *    remaining items from the cache and reallocates the mount point's per-ag
+ *    array, resetting all the counters to zero.
+ *  - The work queue thread resumes and calls the free function for the element
+ *    it started cleaning up earlier.  In the process it decrements the
+ *    filestreams counter for an AG that now has no references.
+ *
+ * With a shrinkfs feature, the above scenario could panic the system.
+ *
+ * All other uses of the following macros should be protected by either the
+ * m_peraglock held in read mode, or the cache's internal locking exposed by the
+ * interval between a call to xfs_mru_cache_lookup() and a call to
+ * xfs_mru_cache_done().  In addition, the m_peraglock must be held in read mode
+ * when new elements are added to the cache.
+ *
+ * Combined, these locking rules ensure that no associations will ever exist in
+ * the cache that reference per-ag array elements that have since been
+ * reallocated.
+ */
+static int
+xfs_filestream_peek_ag(
+	xfs_mount_t	*mp,
+	xfs_agnumber_t	agno)
+{
+	struct xfs_perag *pag;
+	int		ret;
+
+	pag = xfs_perag_get(mp, agno);
+	ret = atomic_read(&pag->pagf_fstrms);
+	xfs_perag_put(pag);
+	return ret;
+}
+
+static int
+xfs_filestream_get_ag(
+	xfs_mount_t	*mp,
+	xfs_agnumber_t	agno)
+{
+	struct xfs_perag *pag;
+	int		ret;
+
+	pag = xfs_perag_get(mp, agno);
+	ret = atomic_inc_return(&pag->pagf_fstrms);
+	xfs_perag_put(pag);
+	return ret;
+}
+
+static void
+xfs_filestream_put_ag(
+	xfs_mount_t	*mp,
+	xfs_agnumber_t	agno)
+{
+	struct xfs_perag *pag;
+
+	pag = xfs_perag_get(mp, agno);
+	atomic_dec(&pag->pagf_fstrms);
+	xfs_perag_put(pag);
+}
 
 /*
  * Scan the AGs starting at startag looking for an AG that isn't in use and has
@@ -351,16 +427,14 @@ xfs_fstrm_free_func(
 {
 	fstrm_item_t	*item  = (fstrm_item_t *)data;
 	xfs_inode_t	*ip = item->ip;
-	int ref;
 
 	ASSERT(ip->i_ino == ino);
 
 	xfs_iflags_clear(ip, XFS_IFILESTREAM);
 
 	/* Drop the reference taken on the AG when the item was added. */
-	ref = xfs_filestream_put_ag(ip->i_mount, item->ag);
+	xfs_filestream_put_ag(ip->i_mount, item->ag);
 
-	ASSERT(ref >= 0);
 	TRACE_FREE(ip->i_mount, ip, item->pip, item->ag,
 		xfs_filestream_peek_ag(ip->i_mount, item->ag));
 
Index: xfs/fs/xfs/xfs_filestream.h
===================================================================
--- xfs.orig/fs/xfs/xfs_filestream.h	2010-07-18 15:10:45.676494680 +0200
+++ xfs/fs/xfs/xfs_filestream.h	2010-07-18 15:10:48.212494681 +0200
@@ -42,88 +42,6 @@ extern ktrace_t *xfs_filestreams_trace_b
 
 #endif
 
-/*
- * Allocation group filestream associations are tracked with per-ag atomic
- * counters.  These counters allow _xfs_filestream_pick_ag() to tell whether a
- * particular AG already has active filestreams associated with it. The mount
- * point's m_peraglock is used to protect these counters from per-ag array
- * re-allocation during a growfs operation.  When xfs_growfs_data_private() is
- * about to reallocate the array, it calls xfs_filestream_flush() with the
- * m_peraglock held in write mode.
- *
- * Since xfs_mru_cache_flush() guarantees that all the free functions for all
- * the cache elements have finished executing before it returns, it's safe for
- * the free functions to use the atomic counters without m_peraglock protection.
- * This allows the implementation of xfs_fstrm_free_func() to be agnostic about
- * whether it was called with the m_peraglock held in read mode, write mode or
- * not held at all.  The race condition this addresses is the following:
- *
- *  - The work queue scheduler fires and pulls a filestream directory cache
- *    element off the LRU end of the cache for deletion, then gets pre-empted.
- *  - A growfs operation grabs the m_peraglock in write mode, flushes all the
- *    remaining items from the cache and reallocates the mount point's per-ag
- *    array, resetting all the counters to zero.
- *  - The work queue thread resumes and calls the free function for the element
- *    it started cleaning up earlier.  In the process it decrements the
- *    filestreams counter for an AG that now has no references.
- *
- * With a shrinkfs feature, the above scenario could panic the system.
- *
- * All other uses of the following macros should be protected by either the
- * m_peraglock held in read mode, or the cache's internal locking exposed by the
- * interval between a call to xfs_mru_cache_lookup() and a call to
- * xfs_mru_cache_done().  In addition, the m_peraglock must be held in read mode
- * when new elements are added to the cache.
- *
- * Combined, these locking rules ensure that no associations will ever exist in
- * the cache that reference per-ag array elements that have since been
- * reallocated.
- */
-/*
- * xfs_filestream_peek_ag is only used in tracing code
- */
-static inline int
-xfs_filestream_peek_ag(
-	xfs_mount_t	*mp,
-	xfs_agnumber_t	agno)
-{
-	struct xfs_perag *pag;
-	int		ret;
-
-	pag = xfs_perag_get(mp, agno);
-	ret = atomic_read(&pag->pagf_fstrms);
-	xfs_perag_put(pag);
-	return ret;
-}
-
-static inline int
-xfs_filestream_get_ag(
-	xfs_mount_t	*mp,
-	xfs_agnumber_t	agno)
-{
-	struct xfs_perag *pag;
-	int		ret;
-
-	pag = xfs_perag_get(mp, agno);
-	ret = atomic_inc_return(&pag->pagf_fstrms);
-	xfs_perag_put(pag);
-	return ret;
-}
-
-static inline int
-xfs_filestream_put_ag(
-	xfs_mount_t	*mp,
-	xfs_agnumber_t	agno)
-{
-	struct xfs_perag *pag;
-	int		ret;
-
-	pag = xfs_perag_get(mp, agno);
-	ret = atomic_dec_return(&pag->pagf_fstrms);
-	xfs_perag_put(pag);
-	return ret;
-}
-
 /* allocation selection flags */
 typedef enum xfs_fstrm_alloc {
 	XFS_PICK_USERDATA = 1,

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

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

* [PATCH 3/6] xfs: remove obsolete osyncisosync mount option
  2010-07-18 20:38 [PATCH 0/6] a few cleanups for 2.6.36 Christoph Hellwig
  2010-07-18 20:38 ` [PATCH 1/6] xfs: fix gcc 4.6 set but not read and unused statement warnings Christoph Hellwig
  2010-07-18 20:38 ` [PATCH 2/6] xfs: clean up filestreams helpers Christoph Hellwig
@ 2010-07-18 20:38 ` Christoph Hellwig
  2010-07-20  7:38   ` Dave Chinner
  2010-07-18 20:38 ` [PATCH 4/6] xfs: kill the b_strat callback in xfs_buf Christoph Hellwig
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Christoph Hellwig @ 2010-07-18 20:38 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: xfs-remove-osyncisosync --]
[-- Type: text/plain, Size: 4447 bytes --]

Since Linux 2.6.33 the kernel has support for real O_SYNC, which made
the osyncisosync option a no-op.  Warn the users about this and remove
the mount flag for it.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs/fs/xfs/linux-2.6/xfs_super.c
===================================================================
--- xfs.orig/fs/xfs/linux-2.6/xfs_super.c	2010-07-18 15:10:45.560494680 +0200
+++ xfs/fs/xfs/linux-2.6/xfs_super.c	2010-07-18 15:14:30.563494681 +0200
@@ -90,7 +90,6 @@ mempool_t *xfs_ioend_pool;
 #define MNTOPT_BARRIER	"barrier"	/* use writer barriers for log write and
 					 * unwritten extent conversion */
 #define MNTOPT_NOBARRIER "nobarrier"	/* .. disable */
-#define MNTOPT_OSYNCISOSYNC "osyncisosync" /* o_sync is REALLY o_sync */
 #define MNTOPT_64BITINODE   "inode64"	/* inodes can be allocated anywhere */
 #define MNTOPT_IKEEP	"ikeep"		/* do not free empty inode clusters */
 #define MNTOPT_NOIKEEP	"noikeep"	/* free empty inode clusters */
@@ -274,8 +273,6 @@ xfs_parseargs(
 			mp->m_flags &= ~XFS_MOUNT_GRPID;
 		} else if (!strcmp(this_char, MNTOPT_WSYNC)) {
 			mp->m_flags |= XFS_MOUNT_WSYNC;
-		} else if (!strcmp(this_char, MNTOPT_OSYNCISOSYNC)) {
-			mp->m_flags |= XFS_MOUNT_OSYNCISOSYNC;
 		} else if (!strcmp(this_char, MNTOPT_NORECOVERY)) {
 			mp->m_flags |= XFS_MOUNT_NORECOVERY;
 		} else if (!strcmp(this_char, MNTOPT_NOALIGN)) {
@@ -366,9 +363,11 @@ xfs_parseargs(
 			cmn_err(CE_WARN,
 	"XFS: ihashsize no longer used, option is deprecated.");
 		} else if (!strcmp(this_char, "osyncisdsync")) {
-			/* no-op, this is now the default */
 			cmn_err(CE_WARN,
-	"XFS: osyncisdsync is now the default, option is deprecated.");
+	"XFS: the osyncisdsync option is deprecated as has no effect.");
+		} else if (!strcmp(this_char, "osyncisosync")) {
+			cmn_err(CE_WARN,
+	"XFS: the osyncisosync option is deprecated as has no effect.");
 		} else if (!strcmp(this_char, "irixsgid")) {
 			cmn_err(CE_WARN,
 	"XFS: irixsgid is now a sysctl(2) variable, option is deprecated.");
@@ -500,7 +499,6 @@ xfs_showargs(
 		{ XFS_MOUNT_SWALLOC,		"," MNTOPT_SWALLOC },
 		{ XFS_MOUNT_NOUUID,		"," MNTOPT_NOUUID },
 		{ XFS_MOUNT_NORECOVERY,		"," MNTOPT_NORECOVERY },
-		{ XFS_MOUNT_OSYNCISOSYNC,	"," MNTOPT_OSYNCISOSYNC },
 		{ XFS_MOUNT_ATTR2,		"," MNTOPT_ATTR2 },
 		{ XFS_MOUNT_FILESTREAMS,	"," MNTOPT_FILESTREAM },
 		{ XFS_MOUNT_GRPID,		"," MNTOPT_GRPID },
Index: xfs/fs/xfs/xfs_mount.h
===================================================================
--- xfs.orig/fs/xfs/xfs_mount.h	2010-07-18 15:10:45.574494680 +0200
+++ xfs/fs/xfs/xfs_mount.h	2010-07-18 15:11:29.884494680 +0200
@@ -220,8 +220,6 @@ typedef struct xfs_mount {
 #define XFS_MOUNT_GRPID		(1ULL << 9)	/* group-ID assigned from directory */
 #define XFS_MOUNT_NORECOVERY	(1ULL << 10)	/* no recovery - dirty fs */
 #define XFS_MOUNT_DFLT_IOSIZE	(1ULL << 12)	/* set default i/o size */
-#define XFS_MOUNT_OSYNCISOSYNC	(1ULL << 13)	/* o_sync is REALLY o_sync */
-						/* osyncisdsync is now default*/
 #define XFS_MOUNT_32BITINODES	(1ULL << 14)	/* do not create inodes above
 						 * 32 bits in size */
 #define XFS_MOUNT_SMALL_INUMS	(1ULL << 15)	/* users wants 32bit inodes */
Index: xfs/Documentation/filesystems/xfs.txt
===================================================================
--- xfs.orig/Documentation/filesystems/xfs.txt	2010-07-18 15:10:45.586494680 +0200
+++ xfs/Documentation/filesystems/xfs.txt	2010-07-18 15:11:29.893494680 +0200
@@ -131,17 +131,6 @@ When mounting an XFS filesystem, the fol
 	Don't check for double mounted file systems using the file system uuid.
 	This is useful to mount LVM snapshot volumes.
 
-  osyncisosync
-	Make O_SYNC writes implement true O_SYNC.  WITHOUT this option,
-	Linux XFS behaves as if an "osyncisdsync" option is used,
-	which will make writes to files opened with the O_SYNC flag set
-	behave as if the O_DSYNC flag had been used instead.
-	This can result in better performance without compromising
-	data safety.
-	However if this option is not in effect, timestamp updates from
-	O_SYNC writes can be lost if the system crashes.
-	If timestamp updates are critical, use the osyncisosync option.
-
   uquota/usrquota/uqnoenforce/quota
 	User disk quota accounting enabled, and limits (optionally)
 	enforced.  Refer to xfs_quota(8) for further details.

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

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

* [PATCH 4/6] xfs: kill the b_strat callback in xfs_buf
  2010-07-18 20:38 [PATCH 0/6] a few cleanups for 2.6.36 Christoph Hellwig
                   ` (2 preceding siblings ...)
  2010-07-18 20:38 ` [PATCH 3/6] xfs: remove obsolete osyncisosync mount option Christoph Hellwig
@ 2010-07-18 20:38 ` Christoph Hellwig
  2010-07-20  7:41   ` Dave Chinner
  2010-07-18 20:38 ` [PATCH 5/6] xfs: simplify xfs_truncate_file Christoph Hellwig
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Christoph Hellwig @ 2010-07-18 20:38 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: xfs-kill-b_strat --]
[-- Type: text/plain, Size: 5577 bytes --]

The b_strat callback is used by xfs_buf_iostrategy to perform additional
checks before submitting a buffer.  It is used in xfs_bwrite and when
writing out delayed buffers.  In xfs_bwrite it we can de-virtualize the
call easily as b_strat is set a few lines above the call to
xfs_buf_iostrategy.  For the delayed buffers the rationale is a bit
more complicated:

 - there are three callers of xfs_buf_delwri_queue, which places buffers
   on the delwri list:
    (1) xfs_bdwrite - this sets up b_strat, so it's fine
    (2) xfs_buf_iorequest.  None of the callers can have XBF_DELWRI set:
	- xlog_bdstrat is only used for log buffers, which are never delwri
	- _xfs_buf_read explicitly clears the delwri flag
	- xfs_buf_iodone_work retries log buffers only
	- xfsbdstrat - only used for reads, superblock writes without the
	  delwri flag, log I/O and file zeroing with explicitly allocated
	  buffers.
	- xfs_buf_iostrategy - only calls xfs_buf_iorequest if b_strat is
	  not set
    (3) xfs_buf_unlock
	- only puts the buffer on the delwri list if the DELWRI flag is
	  already set.  The DELWRI flag is only ever set in xfs_bwrite,
	  xfs_buf_iodone_callbacks, or xfs_trans_log_buf.  For
	  xfs_buf_iodone_callbacks and xfs_trans_log_buf we require
	  an initialized buf item, which means b_strat was set to
	  xfs_bdstrat_cb in xfs_buf_item_init.

Conclusion: we can just get rid of the callback and replace it with
explicit calls to xfs_bdstrat_cb.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs/fs/xfs/linux-2.6/xfs_buf.c
===================================================================
--- xfs.orig/fs/xfs/linux-2.6/xfs_buf.c	2010-07-18 07:16:09.101494681 +0200
+++ xfs/fs/xfs/linux-2.6/xfs_buf.c	2010-07-18 07:16:10.646494682 +0200
@@ -987,13 +987,12 @@ xfs_bwrite(
 {
 	int			error;
 
-	bp->b_strat = xfs_bdstrat_cb;
 	bp->b_mount = mp;
 	bp->b_flags |= XBF_WRITE;
 	bp->b_flags &= ~(XBF_ASYNC | XBF_READ);
 
 	xfs_buf_delwri_dequeue(bp);
-	xfs_buf_iostrategy(bp);
+	xfs_bdstrat_cb(bp);
 
 	error = xfs_buf_iowait(bp);
 	if (error)
@@ -1009,7 +1008,6 @@ xfs_bdwrite(
 {
 	trace_xfs_buf_bdwrite(bp, _RET_IP_);
 
-	bp->b_strat = xfs_bdstrat_cb;
 	bp->b_mount = mp;
 
 	bp->b_flags &= ~XBF_READ;
@@ -1044,7 +1042,6 @@ xfs_bioerror(
 	XFS_BUF_UNDONE(bp);
 	XFS_BUF_STALE(bp);
 
-	XFS_BUF_CLR_BDSTRAT_FUNC(bp);
 	xfs_biodone(bp);
 
 	return EIO;
@@ -1074,7 +1071,6 @@ xfs_bioerror_relse(
 	XFS_BUF_DONE(bp);
 	XFS_BUF_STALE(bp);
 	XFS_BUF_CLR_IODONE_FUNC(bp);
-	XFS_BUF_CLR_BDSTRAT_FUNC(bp);
 	if (!(fl & XBF_ASYNC)) {
 		/*
 		 * Mark b_error and B_ERROR _both_.
@@ -1857,7 +1853,7 @@ xfsbufd(
 			struct xfs_buf *bp;
 			bp = list_first_entry(&tmp, struct xfs_buf, b_list);
 			list_del_init(&bp->b_list);
-			xfs_buf_iostrategy(bp);
+			xfs_bdstrat_cb(bp);
 			count++;
 		}
 		if (count)
@@ -1904,7 +1900,7 @@ xfs_flush_buftarg(
 			bp->b_flags &= ~XBF_ASYNC;
 			list_add(&bp->b_list, &wait_list);
 		}
-		xfs_buf_iostrategy(bp);
+		xfs_bdstrat_cb(bp);
 	}
 
 	if (wait) {
Index: xfs/fs/xfs/linux-2.6/xfs_buf.h
===================================================================
--- xfs.orig/fs/xfs/linux-2.6/xfs_buf.h	2010-07-18 07:16:09.108494681 +0200
+++ xfs/fs/xfs/linux-2.6/xfs_buf.h	2010-07-18 07:16:10.647494682 +0200
@@ -187,7 +187,6 @@ typedef struct xfs_buf {
 	atomic_t		b_io_remaining;	/* #outstanding I/O requests */
 	xfs_buf_iodone_t	b_iodone;	/* I/O completion function */
 	xfs_buf_relse_t		b_relse;	/* releasing function */
-	xfs_buf_bdstrat_t	b_strat;	/* pre-write function */
 	struct completion	b_iowait;	/* queue for I/O waiters */
 	void			*b_fspriv;
 	void			*b_fspriv2;
@@ -245,11 +244,6 @@ extern int xfs_buf_iowait(xfs_buf_t *);
 extern void xfs_buf_iomove(xfs_buf_t *, size_t, size_t, void *,
 				xfs_buf_rw_t);
 
-static inline int xfs_buf_iostrategy(xfs_buf_t *bp)
-{
-	return bp->b_strat ? bp->b_strat(bp) : xfs_buf_iorequest(bp);
-}
-
 static inline int xfs_buf_geterror(xfs_buf_t *bp)
 {
 	return bp ? bp->b_error : ENOMEM;
@@ -321,8 +315,6 @@ extern void xfs_buf_terminate(void);
 #define XFS_BUF_IODONE_FUNC(bp)			((bp)->b_iodone)
 #define XFS_BUF_SET_IODONE_FUNC(bp, func)	((bp)->b_iodone = (func))
 #define XFS_BUF_CLR_IODONE_FUNC(bp)		((bp)->b_iodone = NULL)
-#define XFS_BUF_SET_BDSTRAT_FUNC(bp, func)	((bp)->b_strat = (func))
-#define XFS_BUF_CLR_BDSTRAT_FUNC(bp)		((bp)->b_strat = NULL)
 
 #define XFS_BUF_FSPRIVATE(bp, type)		((type)(bp)->b_fspriv)
 #define XFS_BUF_SET_FSPRIVATE(bp, val)		((bp)->b_fspriv = (void*)(val))
Index: xfs/fs/xfs/xfs_buf_item.c
===================================================================
--- xfs.orig/fs/xfs/xfs_buf_item.c	2010-07-18 07:16:09.117494681 +0200
+++ xfs/fs/xfs/xfs_buf_item.c	2010-07-18 07:16:10.654494682 +0200
@@ -694,7 +694,6 @@ xfs_buf_item_init(
 	 */
 	if (bp->b_mount != mp)
 		bp->b_mount = mp;
-	XFS_BUF_SET_BDSTRAT_FUNC(bp, xfs_bdstrat_cb);
 	if (XFS_BUF_FSPRIVATE(bp, void *) != NULL) {
 		lip = XFS_BUF_FSPRIVATE(bp, xfs_log_item_t *);
 		if (lip->li_type == XFS_LI_BUF) {
Index: xfs/fs/xfs/xfs_inode.c
===================================================================
--- xfs.orig/fs/xfs/xfs_inode.c	2010-07-18 07:16:09.127494681 +0200
+++ xfs/fs/xfs/xfs_inode.c	2010-07-18 07:16:10.662494682 +0200
@@ -2717,7 +2717,6 @@ cluster_corrupt_out:
 		 * mark it as stale and brelse.
 		 */
 		if (XFS_BUF_IODONE_FUNC(bp)) {
-			XFS_BUF_CLR_BDSTRAT_FUNC(bp);
 			XFS_BUF_UNDONE(bp);
 			XFS_BUF_STALE(bp);
 			XFS_BUF_ERROR(bp,EIO);

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

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

* [PATCH 5/6] xfs: simplify xfs_truncate_file
  2010-07-18 20:38 [PATCH 0/6] a few cleanups for 2.6.36 Christoph Hellwig
                   ` (3 preceding siblings ...)
  2010-07-18 20:38 ` [PATCH 4/6] xfs: kill the b_strat callback in xfs_buf Christoph Hellwig
@ 2010-07-18 20:38 ` Christoph Hellwig
  2010-07-20  7:44   ` Dave Chinner
  2010-07-18 20:38 ` [PATCH 6/6] xfs: clean up xfs_bmap_get_bp Christoph Hellwig
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Christoph Hellwig @ 2010-07-18 20:38 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: xfs-simplify-xfs_truncate_file --]
[-- Type: text/plain, Size: 6114 bytes --]

xfs_truncate_file is only used for truncating quota files.  Move it to
xfs_qm_syscalls.c so it can be marked static and take advatange of the
fact by removing the unused page cache validation and taking the iget
into the helper.

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs/fs/xfs/quota/xfs_qm_syscalls.c
===================================================================
--- xfs.orig/fs/xfs/quota/xfs_qm_syscalls.c	2010-06-19 22:35:53.198254580 +0200
+++ xfs/fs/xfs/quota/xfs_qm_syscalls.c	2010-06-19 22:36:27.215004336 +0200
@@ -238,40 +238,74 @@ out_unlock:
 	return error;
 }
 
+STATIC int
+xfs_qm_scall_trunc_qfile(
+	struct xfs_mount	*mp,
+	xfs_ino_t		ino)
+{
+	struct xfs_inode	*ip;
+	struct xfs_trans	*tp;
+	int			error;
+
+	if (ino == NULLFSINO)
+		return 0;
+
+	error = xfs_iget(mp, NULL, ino, 0, 0, &ip);
+	if (error)
+		return error;
+
+	xfs_ilock(ip, XFS_IOLOCK_EXCL);
+
+	tp = xfs_trans_alloc(mp, XFS_TRANS_TRUNCATE_FILE);
+	error = xfs_trans_reserve(tp, 0, XFS_ITRUNCATE_LOG_RES(mp), 0,
+				  XFS_TRANS_PERM_LOG_RES,
+				  XFS_ITRUNCATE_LOG_COUNT);
+	if (error) {
+		xfs_trans_cancel(tp, 0);
+		xfs_iunlock(ip, XFS_IOLOCK_EXCL);
+		goto out_put;
+	}
+
+	xfs_ilock(ip, XFS_ILOCK_EXCL);
+	xfs_trans_ijoin(tp, ip);
+
+	error = xfs_itruncate_finish(&tp, ip, 0, XFS_DATA_FORK, 1);
+	if (error) {
+		xfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES |
+				     XFS_TRANS_ABORT);
+		goto out_unlock;
+	}
+
+	xfs_ichgtime(ip, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
+	error = xfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES);
+
+out_unlock:
+	xfs_iunlock(ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);
+out_put:
+	IRELE(ip);
+	return error;
+}
+
 int
 xfs_qm_scall_trunc_qfiles(
 	xfs_mount_t	*mp,
 	uint		flags)
 {
 	int		error = 0, error2 = 0;
-	xfs_inode_t	*qip;
 
 	if (!xfs_sb_version_hasquota(&mp->m_sb) || flags == 0) {
 		qdprintk("qtrunc flags=%x m_qflags=%x\n", flags, mp->m_qflags);
 		return XFS_ERROR(EINVAL);
 	}
 
-	if ((flags & XFS_DQ_USER) && mp->m_sb.sb_uquotino != NULLFSINO) {
-		error = xfs_iget(mp, NULL, mp->m_sb.sb_uquotino, 0, 0, &qip);
-		if (!error) {
-			error = xfs_truncate_file(mp, qip);
-			IRELE(qip);
-		}
-	}
-
-	if ((flags & (XFS_DQ_GROUP|XFS_DQ_PROJ)) &&
-	    mp->m_sb.sb_gquotino != NULLFSINO) {
-		error2 = xfs_iget(mp, NULL, mp->m_sb.sb_gquotino, 0, 0, &qip);
-		if (!error2) {
-			error2 = xfs_truncate_file(mp, qip);
-			IRELE(qip);
-		}
-	}
+	if (flags & XFS_DQ_USER)
+		error = xfs_qm_scall_trunc_qfile(mp, mp->m_sb.sb_uquotino);
+	if (flags & (XFS_DQ_GROUP|XFS_DQ_PROJ))
+		error2 = xfs_qm_scall_trunc_qfile(mp, mp->m_sb.sb_gquotino);
 
 	return error ? error : error2;
 }
 
-
 /*
  * Switch on (a given) quota enforcement for a filesystem.  This takes
  * effect immediately.
Index: xfs/fs/xfs/xfs_utils.c
===================================================================
--- xfs.orig/fs/xfs/xfs_utils.c	2010-06-19 22:35:53.205004127 +0200
+++ xfs/fs/xfs/xfs_utils.c	2010-06-19 22:36:27.215004336 +0200
@@ -320,86 +320,3 @@ xfs_bumplink(
 	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
 	return 0;
 }
-
-/*
- * Try to truncate the given file to 0 length.  Currently called
- * only out of xfs_remove when it has to truncate a file to free
- * up space for the remove to proceed.
- */
-int
-xfs_truncate_file(
-	xfs_mount_t	*mp,
-	xfs_inode_t	*ip)
-{
-	xfs_trans_t	*tp;
-	int		error;
-
-#ifdef QUOTADEBUG
-	/*
-	 * This is called to truncate the quotainodes too.
-	 */
-	if (XFS_IS_UQUOTA_ON(mp)) {
-		if (ip->i_ino != mp->m_sb.sb_uquotino)
-			ASSERT(ip->i_udquot);
-	}
-	if (XFS_IS_OQUOTA_ON(mp)) {
-		if (ip->i_ino != mp->m_sb.sb_gquotino)
-			ASSERT(ip->i_gdquot);
-	}
-#endif
-	/*
-	 * Make the call to xfs_itruncate_start before starting the
-	 * transaction, because we cannot make the call while we're
-	 * in a transaction.
-	 */
-	xfs_ilock(ip, XFS_IOLOCK_EXCL);
-	error = xfs_itruncate_start(ip, XFS_ITRUNC_DEFINITE, (xfs_fsize_t)0);
-	if (error) {
-		xfs_iunlock(ip, XFS_IOLOCK_EXCL);
-		return error;
-	}
-
-	tp = xfs_trans_alloc(mp, XFS_TRANS_TRUNCATE_FILE);
-	if ((error = xfs_trans_reserve(tp, 0, XFS_ITRUNCATE_LOG_RES(mp), 0,
-				      XFS_TRANS_PERM_LOG_RES,
-				      XFS_ITRUNCATE_LOG_COUNT))) {
-		xfs_trans_cancel(tp, 0);
-		xfs_iunlock(ip, XFS_IOLOCK_EXCL);
-		return error;
-	}
-
-	/*
-	 * Follow the normal truncate locking protocol.  Since we
-	 * hold the inode in the transaction, we know that its number
-	 * of references will stay constant.
-	 */
-	xfs_ilock(ip, XFS_ILOCK_EXCL);
-	xfs_trans_ijoin(tp, ip);
-
-	/*
-	 * Signal a sync xaction.  The only case where that isn't
-	 * the case is if we're truncating an already unlinked file
-	 * on a wsync fs.  In that case, we know the blocks can't
-	 * reappear in the file because the links to file are
-	 * permanently toast.  Currently, we're always going to
-	 * want a sync transaction because this code is being
-	 * called from places where nlink is guaranteed to be 1
-	 * but I'm leaving the tests in to protect against future
-	 * changes -- rcc.
-	 */
-	error = xfs_itruncate_finish(&tp, ip, (xfs_fsize_t)0,
-				     XFS_DATA_FORK,
-				     ((ip->i_d.di_nlink != 0 ||
-				       !(mp->m_flags & XFS_MOUNT_WSYNC))
-				      ? 1 : 0));
-	if (error) {
-		xfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES |
-				 XFS_TRANS_ABORT);
-	} else {
-		xfs_ichgtime(ip, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
-		error = xfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES);
-	}
-	xfs_iunlock(ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);
-
-	return error;
-}
Index: xfs/fs/xfs/xfs_utils.h
===================================================================
--- xfs.orig/fs/xfs/xfs_utils.h	2010-06-19 22:35:53.222003778 +0200
+++ xfs/fs/xfs/xfs_utils.h	2010-06-19 22:36:27.218012367 +0200
@@ -18,7 +18,6 @@
 #ifndef __XFS_UTILS_H__
 #define __XFS_UTILS_H__
 
-extern int xfs_truncate_file(xfs_mount_t *, xfs_inode_t *);
 extern int xfs_dir_ialloc(xfs_trans_t **, xfs_inode_t *, mode_t, xfs_nlink_t,
 				xfs_dev_t, cred_t *, prid_t, int,
 				xfs_inode_t **, int *);

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

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

* [PATCH 6/6] xfs: clean up xfs_bmap_get_bp
  2010-07-18 20:38 [PATCH 0/6] a few cleanups for 2.6.36 Christoph Hellwig
                   ` (4 preceding siblings ...)
  2010-07-18 20:38 ` [PATCH 5/6] xfs: simplify xfs_truncate_file Christoph Hellwig
@ 2010-07-18 20:38 ` Christoph Hellwig
  2010-07-20  7:46   ` Dave Chinner
  2010-07-20  7:50 ` [PATCH 0/6] a few cleanups for 2.6.36 Dave Chinner
  2010-07-21 13:35 ` Alex Elder
  7 siblings, 1 reply; 16+ messages in thread
From: Christoph Hellwig @ 2010-07-18 20:38 UTC (permalink / raw)
  To: xfs

[-- Attachment #1: xfs-cleanup-xfs_bmap_get_bp --]
[-- Type: text/plain, Size: 1814 bytes --]

Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: xfs/fs/xfs/xfs_bmap.c
===================================================================
--- xfs.orig/fs/xfs/xfs_bmap.c	2010-06-03 13:33:14.087004121 +0200
+++ xfs/fs/xfs/xfs_bmap.c	2010-06-03 13:36:46.510005447 +0200
@@ -5648,41 +5648,34 @@ xfs_bmap_eof(
 }
 
 #ifdef DEBUG
-STATIC
-xfs_buf_t *
+STATIC struct xfs_buf *
 xfs_bmap_get_bp(
-	xfs_btree_cur_t         *cur,
+	struct xfs_btree_cur	*cur,
 	xfs_fsblock_t		bno)
 {
-	int i;
-	xfs_buf_t *bp;
+	struct xfs_log_item_desc *lidp;
+	int			i;
 
 	if (!cur)
-		return(NULL);
+		return NULL;
 
-	bp = NULL;
-	for(i = 0; i < XFS_BTREE_MAXLEVELS; i++) {
-		bp = cur->bc_bufs[i];
-		if (!bp) break;
-		if (XFS_BUF_ADDR(bp) == bno)
-			break;	/* Found it */
+	for (i = 0; i < XFS_BTREE_MAXLEVELS; i++) {
+		if (!cur->bc_bufs[i])
+			break;
+		if (XFS_BUF_ADDR(cur->bc_bufs[i]) == bno)
+			return cur->bc_bufs[i];
 	}
-	if (i == XFS_BTREE_MAXLEVELS)
-		bp = NULL;
 
-	if (!bp) { /* Chase down all the log items to see if the bp is there */
-		struct xfs_log_item_desc *lidp;
+	/* Chase down all the log items to see if the bp is there */
+	list_for_each_entry(lidp, &cur->bc_tp->t_items, lid_trans) {
 		struct xfs_buf_log_item	*bip;
-
-		list_for_each_entry(lidp, &cur->bc_tp->t_items, lid_trans) {
-			bip = (struct xfs_buf_log_item *)lidp->lid_item;
-			if (bip->bli_item.li_type == XFS_LI_BUF &&
-			    XFS_BUF_ADDR(bip->bli_buf) == bno)
-				return bip->bli_buf;
-		}
+		bip = (struct xfs_buf_log_item *)lidp->lid_item;
+		if (bip->bli_item.li_type == XFS_LI_BUF &&
+		    XFS_BUF_ADDR(bip->bli_buf) == bno)
+			return bip->bli_buf;
 	}
 
-	return bp;
+	return NULL;
 }
 
 STATIC void

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

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

* Re: [PATCH 1/6] xfs: fix gcc 4.6 set but not read and unused statement warnings
  2010-07-18 20:38 ` [PATCH 1/6] xfs: fix gcc 4.6 set but not read and unused statement warnings Christoph Hellwig
@ 2010-07-20  7:34   ` Dave Chinner
  0 siblings, 0 replies; 16+ messages in thread
From: Dave Chinner @ 2010-07-20  7:34 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Andi Kleen, xfs

On Sun, Jul 18, 2010 at 04:38:49PM -0400, Christoph Hellwig wrote:
> [hch: dropped a few hunks that need structural changes instead]
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

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

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

* Re: [PATCH 2/6] xfs: clean up filestreams helpers
  2010-07-18 20:38 ` [PATCH 2/6] xfs: clean up filestreams helpers Christoph Hellwig
@ 2010-07-20  7:35   ` Dave Chinner
  0 siblings, 0 replies; 16+ messages in thread
From: Dave Chinner @ 2010-07-20  7:35 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Sun, Jul 18, 2010 at 04:38:50PM -0400, Christoph Hellwig wrote:
> Move xfs_filestream_peek_ag, xxfs_filestream_get_ag and xfs_filestream_put_ag
> from xfs_filestream.h to xfs_filestream.c where it's only callers are, and
> remove the inline marker while we're at it to let the compiler decide on the
> inlining.  Also don't return a value from xfs_filestream_put_ag because
> we don't need it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Dave Chinner <david@fromorbit.com>

-- 
Dave Chinner
david@fromorbit.com

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

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

* Re: [PATCH 3/6] xfs: remove obsolete osyncisosync mount option
  2010-07-18 20:38 ` [PATCH 3/6] xfs: remove obsolete osyncisosync mount option Christoph Hellwig
@ 2010-07-20  7:38   ` Dave Chinner
  2010-07-20  8:09     ` Christoph Hellwig
  0 siblings, 1 reply; 16+ messages in thread
From: Dave Chinner @ 2010-07-20  7:38 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Sun, Jul 18, 2010 at 04:38:51PM -0400, Christoph Hellwig wrote:
> Since Linux 2.6.33 the kernel has support for real O_SYNC, which made
> the osyncisosync option a no-op.  Warn the users about this and remove
> the mount flag for it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good, but I think these messages:
> 
> @@ -366,9 +363,11 @@ xfs_parseargs(
>  			cmn_err(CE_WARN,
>  	"XFS: ihashsize no longer used, option is deprecated.");
>  		} else if (!strcmp(this_char, "osyncisdsync")) {
> -			/* no-op, this is now the default */
>  			cmn_err(CE_WARN,
> -	"XFS: osyncisdsync is now the default, option is deprecated.");
> +	"XFS: the osyncisdsync option is deprecated as has no effect.");
> +		} else if (!strcmp(this_char, "osyncisosync")) {
> +			cmn_err(CE_WARN,
> +	"XFS: the osyncisosync option is deprecated as has no effect.");
>  		} else if (!strcmp(this_char, "irixsgid")) {
>  			cmn_err(CE_WARN,
>  	"XFS: irixsgid is now a sysctl(2) variable, option is deprecated.");

Look better as:

	"XFS: osyncisdsync has no effect, option is deprecated."
	"XFS: osyncisosync has no effect, option is deprecated."

so they are consistent with the other deprecated option output.
I've made this change to the patch i have in my tree....

Otherwise,

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

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

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

* Re: [PATCH 4/6] xfs: kill the b_strat callback in xfs_buf
  2010-07-18 20:38 ` [PATCH 4/6] xfs: kill the b_strat callback in xfs_buf Christoph Hellwig
@ 2010-07-20  7:41   ` Dave Chinner
  0 siblings, 0 replies; 16+ messages in thread
From: Dave Chinner @ 2010-07-20  7:41 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Sun, Jul 18, 2010 at 04:38:52PM -0400, Christoph Hellwig wrote:
> The b_strat callback is used by xfs_buf_iostrategy to perform additional
> checks before submitting a buffer.  It is used in xfs_bwrite and when
> writing out delayed buffers.  In xfs_bwrite it we can de-virtualize the
> call easily as b_strat is set a few lines above the call to
> xfs_buf_iostrategy.  For the delayed buffers the rationale is a bit
> more complicated:
> 
>  - there are three callers of xfs_buf_delwri_queue, which places buffers
>    on the delwri list:
>     (1) xfs_bdwrite - this sets up b_strat, so it's fine
>     (2) xfs_buf_iorequest.  None of the callers can have XBF_DELWRI set:
> 	- xlog_bdstrat is only used for log buffers, which are never delwri
> 	- _xfs_buf_read explicitly clears the delwri flag
> 	- xfs_buf_iodone_work retries log buffers only
> 	- xfsbdstrat - only used for reads, superblock writes without the
> 	  delwri flag, log I/O and file zeroing with explicitly allocated
> 	  buffers.
> 	- xfs_buf_iostrategy - only calls xfs_buf_iorequest if b_strat is
> 	  not set
>     (3) xfs_buf_unlock
> 	- only puts the buffer on the delwri list if the DELWRI flag is
> 	  already set.  The DELWRI flag is only ever set in xfs_bwrite,
> 	  xfs_buf_iodone_callbacks, or xfs_trans_log_buf.  For
> 	  xfs_buf_iodone_callbacks and xfs_trans_log_buf we require
> 	  an initialized buf item, which means b_strat was set to
> 	  xfs_bdstrat_cb in xfs_buf_item_init.
> 
> Conclusion: we can just get rid of the callback and replace it with
> explicit calls to xfs_bdstrat_cb.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

looks good.

Reviewed-by: Dave Chinner <david@fromorbit.com>

-- 
Dave Chinner
david@fromorbit.com

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

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

* Re: [PATCH 5/6] xfs: simplify xfs_truncate_file
  2010-07-18 20:38 ` [PATCH 5/6] xfs: simplify xfs_truncate_file Christoph Hellwig
@ 2010-07-20  7:44   ` Dave Chinner
  0 siblings, 0 replies; 16+ messages in thread
From: Dave Chinner @ 2010-07-20  7:44 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Sun, Jul 18, 2010 at 04:38:53PM -0400, Christoph Hellwig wrote:
> xfs_truncate_file is only used for truncating quota files.  Move it to
> xfs_qm_syscalls.c so it can be marked static and take advatange of the
> fact by removing the unused page cache validation and taking the iget
> into the helper.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Dave Chinner <david@fromorbit.com>

-- 
Dave Chinner
david@fromorbit.com

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

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

* Re: [PATCH 6/6] xfs: clean up xfs_bmap_get_bp
  2010-07-18 20:38 ` [PATCH 6/6] xfs: clean up xfs_bmap_get_bp Christoph Hellwig
@ 2010-07-20  7:46   ` Dave Chinner
  0 siblings, 0 replies; 16+ messages in thread
From: Dave Chinner @ 2010-07-20  7:46 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Sun, Jul 18, 2010 at 04:38:54PM -0400, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good - even debug code gets an occasional polish ;)

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

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

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

* Re: [PATCH 0/6] a few cleanups for 2.6.36
  2010-07-18 20:38 [PATCH 0/6] a few cleanups for 2.6.36 Christoph Hellwig
                   ` (5 preceding siblings ...)
  2010-07-18 20:38 ` [PATCH 6/6] xfs: clean up xfs_bmap_get_bp Christoph Hellwig
@ 2010-07-20  7:50 ` Dave Chinner
  2010-07-21 13:35 ` Alex Elder
  7 siblings, 0 replies; 16+ messages in thread
From: Dave Chinner @ 2010-07-20  7:50 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Sun, Jul 18, 2010 at 04:38:48PM -0400, Christoph Hellwig wrote:
> A couple of random cleanups, including those hunks from Andi's gcc 4.6
> warning fixes that can easily be applied.

I'm running QA on this series plus all the other outstanding
patches. I'll push those out into a branch when that is done...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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

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

* Re: [PATCH 3/6] xfs: remove obsolete osyncisosync mount option
  2010-07-20  7:38   ` Dave Chinner
@ 2010-07-20  8:09     ` Christoph Hellwig
  0 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2010-07-20  8:09 UTC (permalink / raw)
  To: Dave Chinner; +Cc: Christoph Hellwig, xfs

On Tue, Jul 20, 2010 at 05:38:32PM +1000, Dave Chinner wrote:
> >  	"XFS: ihashsize no longer used, option is deprecated.");
> >  		} else if (!strcmp(this_char, "osyncisdsync")) {
> > -			/* no-op, this is now the default */
> >  			cmn_err(CE_WARN,
> > -	"XFS: osyncisdsync is now the default, option is deprecated.");
> > +	"XFS: the osyncisdsync option is deprecated as has no effect.");
> > +		} else if (!strcmp(this_char, "osyncisosync")) {
> > +			cmn_err(CE_WARN,
> > +	"XFS: the osyncisosync option is deprecated as has no effect.");
> >  		} else if (!strcmp(this_char, "irixsgid")) {
> >  			cmn_err(CE_WARN,
> >  	"XFS: irixsgid is now a sysctl(2) variable, option is deprecated.");
> 
> Look better as:
> 
> 	"XFS: osyncisdsync has no effect, option is deprecated."
> 	"XFS: osyncisosync has no effect, option is deprecated."
> 
> so they are consistent with the other deprecated option output.
> I've made this change to the patch i have in my tree....

Sure, fine with me.

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

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

* Re: [PATCH 0/6] a few cleanups for 2.6.36
  2010-07-18 20:38 [PATCH 0/6] a few cleanups for 2.6.36 Christoph Hellwig
                   ` (6 preceding siblings ...)
  2010-07-20  7:50 ` [PATCH 0/6] a few cleanups for 2.6.36 Dave Chinner
@ 2010-07-21 13:35 ` Alex Elder
  7 siblings, 0 replies; 16+ messages in thread
From: Alex Elder @ 2010-07-21 13:35 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: xfs

On Sun, 2010-07-18 at 16:38 -0400, Christoph Hellwig wrote:
> A couple of random cleanups, including those hunks from Andi's gcc 4.6
> warning fixes that can easily be applied.
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

This whole series looks good to me.

Reviewed-by: Alex Elder <aelder@sgi.com>


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

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

end of thread, other threads:[~2010-07-21 13:32 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-18 20:38 [PATCH 0/6] a few cleanups for 2.6.36 Christoph Hellwig
2010-07-18 20:38 ` [PATCH 1/6] xfs: fix gcc 4.6 set but not read and unused statement warnings Christoph Hellwig
2010-07-20  7:34   ` Dave Chinner
2010-07-18 20:38 ` [PATCH 2/6] xfs: clean up filestreams helpers Christoph Hellwig
2010-07-20  7:35   ` Dave Chinner
2010-07-18 20:38 ` [PATCH 3/6] xfs: remove obsolete osyncisosync mount option Christoph Hellwig
2010-07-20  7:38   ` Dave Chinner
2010-07-20  8:09     ` Christoph Hellwig
2010-07-18 20:38 ` [PATCH 4/6] xfs: kill the b_strat callback in xfs_buf Christoph Hellwig
2010-07-20  7:41   ` Dave Chinner
2010-07-18 20:38 ` [PATCH 5/6] xfs: simplify xfs_truncate_file Christoph Hellwig
2010-07-20  7:44   ` Dave Chinner
2010-07-18 20:38 ` [PATCH 6/6] xfs: clean up xfs_bmap_get_bp Christoph Hellwig
2010-07-20  7:46   ` Dave Chinner
2010-07-20  7:50 ` [PATCH 0/6] a few cleanups for 2.6.36 Dave Chinner
2010-07-21 13:35 ` Alex Elder

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