From f9057e3da79d18fdbd9d6adbb183f032c614feeb Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 4 Feb 2009 09:31:52 +0100 Subject: xfs: cleanup error handling in xfs_mountfs: Clean up the error handling in xfs_mountfs. Use readable goto label names, simplify the uuid handling and other error conditions. Signed-off-by: Christoph Hellwig Reviewed-by: Felix Blyakher diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c index 3530025..86ac80c 100644 --- a/fs/xfs/xfs_mount.c +++ b/fs/xfs/xfs_mount.c @@ -886,8 +886,6 @@ xfs_check_sizes(xfs_mount_t *mp) } /* - * xfs_mountfs - * * This function does the following on an initial mount of a file system: * - reads the superblock from disk and init the mount struct * - if we're a 32-bit kernel, do a size check on the superblock @@ -905,7 +903,6 @@ xfs_mountfs( xfs_inode_t *rip; __uint64_t resblks; uint quotamount, quotaflags; - int uuid_mounted = 0; int error = 0; xfs_mount_common(mp, sbp); @@ -960,7 +957,7 @@ xfs_mountfs( */ error = xfs_update_alignment(mp); if (error) - goto error1; + goto out; xfs_alloc_compute_maxlevels(mp); xfs_bmap_compute_maxlevels(mp, XFS_DATA_FORK); @@ -977,12 +974,11 @@ xfs_mountfs( * since a single partition filesystem is identical to a single * partition volume/filesystem. */ - if ((mp->m_flags & XFS_MOUNT_NOUUID) == 0) { + if (!(mp->m_flags & XFS_MOUNT_NOUUID)) { if (xfs_uuid_mount(mp)) { error = XFS_ERROR(EINVAL); - goto error1; + goto out; } - uuid_mounted=1; } /* @@ -1007,7 +1003,7 @@ xfs_mountfs( */ error = xfs_check_sizes(mp); if (error) - goto error1; + goto out_remove_uuid; /* * Initialize realtime fields in the mount structure @@ -1015,7 +1011,7 @@ xfs_mountfs( error = xfs_rtmount_init(mp); if (error) { cmn_err(CE_WARN, "XFS: RT mount failed"); - goto error1; + goto out_remove_uuid; } /* @@ -1045,26 +1041,26 @@ xfs_mountfs( mp->m_perag = kmem_zalloc(sbp->sb_agcount * sizeof(xfs_perag_t), KM_MAYFAIL); if (!mp->m_perag) - goto error1; + goto out_remove_uuid; mp->m_maxagi = xfs_initialize_perag(mp, sbp->sb_agcount); + if (!sbp->sb_logblocks) { + cmn_err(CE_WARN, "XFS: no log defined"); + XFS_ERROR_REPORT("xfs_mountfs", XFS_ERRLEVEL_LOW, mp); + error = XFS_ERROR(EFSCORRUPTED); + goto out_free_perag; + } + /* * log's mount-time initialization. Perform 1st part recovery if needed */ - if (likely(sbp->sb_logblocks > 0)) { /* check for volume case */ - error = xfs_log_mount(mp, mp->m_logdev_targp, - XFS_FSB_TO_DADDR(mp, sbp->sb_logstart), - XFS_FSB_TO_BB(mp, sbp->sb_logblocks)); - if (error) { - cmn_err(CE_WARN, "XFS: log mount failed"); - goto error2; - } - } else { /* No log has been defined */ - cmn_err(CE_WARN, "XFS: no log defined"); - XFS_ERROR_REPORT("xfs_mountfs_int(1)", XFS_ERRLEVEL_LOW, mp); - error = XFS_ERROR(EFSCORRUPTED); - goto error2; + error = xfs_log_mount(mp, mp->m_logdev_targp, + XFS_FSB_TO_DADDR(mp, sbp->sb_logstart), + XFS_FSB_TO_BB(mp, sbp->sb_logblocks)); + if (error) { + cmn_err(CE_WARN, "XFS: log mount failed"); + goto out_free_perag; } /* @@ -1086,15 +1082,14 @@ xfs_mountfs( * If we are currently making the filesystem, the initialisation will * fail as the perag data is in an undefined state. */ - if (xfs_sb_version_haslazysbcount(&mp->m_sb) && !XFS_LAST_UNMOUNT_WAS_CLEAN(mp) && !mp->m_sb.sb_inprogress) { error = xfs_initialize_perag_data(mp, sbp->sb_agcount); - if (error) { - goto error2; - } + if (error) + goto out_free_perag; } + /* * Get and sanity-check the root inode. * Save the pointer to it in the mount structure. @@ -1102,7 +1097,7 @@ xfs_mountfs( error = xfs_iget(mp, NULL, sbp->sb_rootino, 0, XFS_ILOCK_EXCL, &rip, 0); if (error) { cmn_err(CE_WARN, "XFS: failed to read root inode"); - goto error3; + goto out_log_dealloc; } ASSERT(rip != NULL); @@ -1116,7 +1111,7 @@ xfs_mountfs( XFS_ERROR_REPORT("xfs_mountfs_int(2)", XFS_ERRLEVEL_LOW, mp); error = XFS_ERROR(EFSCORRUPTED); - goto error4; + goto out_rele_rip; } mp->m_rootip = rip; /* save it */ @@ -1131,7 +1126,7 @@ xfs_mountfs( * Free up the root inode. */ cmn_err(CE_WARN, "XFS: failed to read RT inodes"); - goto error4; + goto out_rele_rip; } /* @@ -1143,7 +1138,7 @@ xfs_mountfs( error = xfs_mount_log_sb(mp, mp->m_update_flags); if (error) { cmn_err(CE_WARN, "XFS: failed to write sb changes"); - goto error4; + goto out_rele_rip; } } @@ -1152,7 +1147,7 @@ xfs_mountfs( */ error = XFS_QM_INIT(mp, "amount, "aflags); if (error) - goto error4; + goto out_rele_rip; /* * Finish recovering the file system. This part needed to be @@ -1162,7 +1157,7 @@ xfs_mountfs( error = xfs_log_mount_finish(mp); if (error) { cmn_err(CE_WARN, "XFS: log mount finish failed"); - goto error4; + goto out_rele_rip; } /* @@ -1170,7 +1165,7 @@ xfs_mountfs( */ error = XFS_QM_MOUNT(mp, quotamount, quotaflags); if (error) - goto error4; + goto out_rele_rip; /* * Now we are mounted, reserve a small amount of unused space for @@ -1194,18 +1189,16 @@ xfs_mountfs( return 0; - error4: - /* - * Free up the root inode. - */ + out_rele_rip: IRELE(rip); - error3: + out_log_dealloc: xfs_log_unmount_dealloc(mp); - error2: + out_free_perag: xfs_free_perag(mp); - error1: - if (uuid_mounted) + out_remove_uuid: + if (!(mp->m_flags & XFS_MOUNT_NOUUID)) uuid_table_remove(&mp->m_sb.sb_uuid); + out: return error; } -- cgit v0.10.2 From b93b6e434c046459cf3111c76dce46ba4abcb2b6 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 4 Feb 2009 09:33:58 +0100 Subject: xfs: make sure to free the real-time inodes in the mount error path When mount fails after allocating the real-time inodes we currently leak them. Add a new helper to free the real-time inodes which can be used by both the mount and unmount path. Signed-off-by: Christoph Hellwig Reviewed-by: Felix Blyakher diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c index 86ac80c..664961e 100644 --- a/fs/xfs/xfs_mount.c +++ b/fs/xfs/xfs_mount.c @@ -1138,7 +1138,7 @@ xfs_mountfs( error = xfs_mount_log_sb(mp, mp->m_update_flags); if (error) { cmn_err(CE_WARN, "XFS: failed to write sb changes"); - goto out_rele_rip; + goto out_rtunmount; } } @@ -1147,7 +1147,7 @@ xfs_mountfs( */ error = XFS_QM_INIT(mp, "amount, "aflags); if (error) - goto out_rele_rip; + goto out_rtunmount; /* * Finish recovering the file system. This part needed to be @@ -1157,7 +1157,7 @@ xfs_mountfs( error = xfs_log_mount_finish(mp); if (error) { cmn_err(CE_WARN, "XFS: log mount finish failed"); - goto out_rele_rip; + goto out_rtunmount; } /* @@ -1165,7 +1165,7 @@ xfs_mountfs( */ error = XFS_QM_MOUNT(mp, quotamount, quotaflags); if (error) - goto out_rele_rip; + goto out_rtunmount; /* * Now we are mounted, reserve a small amount of unused space for @@ -1189,6 +1189,8 @@ xfs_mountfs( return 0; + out_rtunmount: + xfs_rtunmount_inodes(mp); out_rele_rip: IRELE(rip); out_log_dealloc: @@ -1219,10 +1221,7 @@ xfs_unmountfs( */ XFS_QM_UNMOUNT(mp); - if (mp->m_rbmip) - IRELE(mp->m_rbmip); - if (mp->m_rsumip) - IRELE(mp->m_rsumip); + xfs_rtunmount_inodes(mp); IRELE(mp->m_rootip); /* diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c index c5bb86f..385f6dc 100644 --- a/fs/xfs/xfs_rtalloc.c +++ b/fs/xfs/xfs_rtalloc.c @@ -2288,6 +2288,16 @@ xfs_rtmount_inodes( return 0; } +void +xfs_rtunmount_inodes( + struct xfs_mount *mp) +{ + if (mp->m_rbmip) + IRELE(mp->m_rbmip); + if (mp->m_rsumip) + IRELE(mp->m_rsumip); +} + /* * Pick an extent for allocation at the start of a new realtime file. * Use the sequence number stored in the atime field of the bitmap inode. diff --git a/fs/xfs/xfs_rtalloc.h b/fs/xfs/xfs_rtalloc.h index 8d8dcd2..3bac681 100644 --- a/fs/xfs/xfs_rtalloc.h +++ b/fs/xfs/xfs_rtalloc.h @@ -108,6 +108,9 @@ xfs_rtfree_extent( int /* error */ xfs_rtmount_init( struct xfs_mount *mp); /* file system mount structure */ +void +xfs_rtunmount_inodes( + struct xfs_mount *mp); /* * Get the bitmap and summary inodes into the mount structure @@ -146,6 +149,7 @@ xfs_growfs_rt( # define xfs_growfs_rt(mp,in) (ENOSYS) # define xfs_rtmount_init(m) (((mp)->m_sb.sb_rblocks == 0)? 0 : (ENOSYS)) # define xfs_rtmount_inodes(m) (((mp)->m_sb.sb_rblocks == 0)? 0 : (ENOSYS)) +# define xfs_rtunmount_inodes(m) #endif /* CONFIG_XFS_RT */ #endif /* __KERNEL__ */ -- cgit v0.10.2 From cb3f35bb3bf0759e00cd4f68155da9b636421f84 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 4 Feb 2009 09:34:20 +0100 Subject: xfs: tiny cleanup for xfs_link The source and target inodes are guaranteed to never be the same by the VFS, so no need to check for that (and we would get into bad trouble later anyway if that were the case). Also clean up the error handling to use two gotos instead of nested conditions. Signed-off-by: Christoph Hellwig Reviewed-by: Felix Blyakher diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c index 0e55c5d..4229408 100644 --- a/fs/xfs/xfs_vnodeops.c +++ b/fs/xfs/xfs_vnodeops.c @@ -2004,8 +2004,10 @@ xfs_link( /* Return through std_return after this point. */ error = XFS_QM_DQATTACH(mp, sip, 0); - if (!error && sip != tdp) - error = XFS_QM_DQATTACH(mp, tdp, 0); + if (error) + goto std_return; + + error = XFS_QM_DQATTACH(mp, tdp, 0); if (error) goto std_return; -- cgit v0.10.2 From c52e9fd8a9d3ac019680ffa315c1a0689d401ce3 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 4 Feb 2009 09:34:34 +0100 Subject: xfs: remove unused XFS_MOUNT_ILOCK/XFS_MOUNT_IUNLOCK These aren't only unused but also reference a lock that doesn't exist anymore. Signed-off-by: Christoph Hellwig Reviewed-by: Felix Blyakher diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h index f5e9937..670c10e0 100644 --- a/fs/xfs/xfs_mount.h +++ b/fs/xfs/xfs_mount.h @@ -500,9 +500,6 @@ typedef struct xfs_mod_sb { int64_t msb_delta; /* Change to make to specified field */ } xfs_mod_sb_t; -#define XFS_MOUNT_ILOCK(mp) mutex_lock(&((mp)->m_ilock)) -#define XFS_MOUNT_IUNLOCK(mp) mutex_unlock(&((mp)->m_ilock)) - extern int xfs_log_sbcount(xfs_mount_t *, uint); extern int xfs_mountfs(xfs_mount_t *mp); extern void xfs_mountfs_check_barriers(xfs_mount_t *mp); -- cgit v0.10.2 From e1486dea0bf4bc75a52a983281076f454a894b66 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 4 Feb 2009 09:36:00 +0100 Subject: xfs: factor out attr fork reset handling We currently duplicate code to reset the attribute fork after the last attribute has been deleted. Factor this out into a small helper. Signed-off-by: Christoph Hellwig Reviewed-by: Felix Blyakher diff --git a/fs/xfs/xfs_attr_leaf.c b/fs/xfs/xfs_attr_leaf.c index 6c323f8..aa00162 100644 --- a/fs/xfs/xfs_attr_leaf.c +++ b/fs/xfs/xfs_attr_leaf.c @@ -298,6 +298,26 @@ xfs_attr_shortform_add(xfs_da_args_t *args, int forkoff) } /* + * After the last attribute is removed revert to original inode format, + * making all literal area available to the data fork once more. + */ +STATIC void +xfs_attr_fork_reset( + struct xfs_inode *ip, + struct xfs_trans *tp) +{ + xfs_idestroy_fork(ip, XFS_ATTR_FORK); + ip->i_d.di_forkoff = 0; + ip->i_d.di_aformat = XFS_DINODE_FMT_EXTENTS; + + ASSERT(ip->i_d.di_anextents == 0); + ASSERT(ip->i_afp == NULL); + + ip->i_df.if_ext_max = XFS_IFORK_DSIZE(ip) / sizeof(xfs_bmbt_rec_t); + xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); +} + +/* * Remove an attribute from the shortform attribute list structure. */ int @@ -344,22 +364,10 @@ xfs_attr_shortform_remove(xfs_da_args_t *args) */ totsize -= size; if (totsize == sizeof(xfs_attr_sf_hdr_t) && - !(args->op_flags & XFS_DA_OP_ADDNAME) && - (mp->m_flags & XFS_MOUNT_ATTR2) && - (dp->i_d.di_format != XFS_DINODE_FMT_BTREE)) { - /* - * Last attribute now removed, revert to original - * inode format making all literal area available - * to the data fork once more. - */ - xfs_idestroy_fork(dp, XFS_ATTR_FORK); - dp->i_d.di_forkoff = 0; - dp->i_d.di_aformat = XFS_DINODE_FMT_EXTENTS; - ASSERT(dp->i_d.di_anextents == 0); - ASSERT(dp->i_afp == NULL); - dp->i_df.if_ext_max = - XFS_IFORK_DSIZE(dp) / (uint)sizeof(xfs_bmbt_rec_t); - xfs_trans_log_inode(args->trans, dp, XFS_ILOG_CORE); + (mp->m_flags & XFS_MOUNT_ATTR2) && + (dp->i_d.di_format != XFS_DINODE_FMT_BTREE) && + !(args->op_flags & XFS_DA_OP_ADDNAME)) { + xfs_attr_fork_reset(dp, args->trans); } else { xfs_idata_realloc(dp, -size, XFS_ATTR_FORK); dp->i_d.di_forkoff = xfs_attr_shortform_bytesfit(dp, totsize); @@ -786,20 +794,7 @@ xfs_attr_leaf_to_shortform(xfs_dabuf_t *bp, xfs_da_args_t *args, int forkoff) if (forkoff == -1) { ASSERT(dp->i_mount->m_flags & XFS_MOUNT_ATTR2); ASSERT(dp->i_d.di_format != XFS_DINODE_FMT_BTREE); - - /* - * Last attribute was removed, revert to original - * inode format making all literal area available - * to the data fork once more. - */ - xfs_idestroy_fork(dp, XFS_ATTR_FORK); - dp->i_d.di_forkoff = 0; - dp->i_d.di_aformat = XFS_DINODE_FMT_EXTENTS; - ASSERT(dp->i_d.di_anextents == 0); - ASSERT(dp->i_afp == NULL); - dp->i_df.if_ext_max = - XFS_IFORK_DSIZE(dp) / (uint)sizeof(xfs_bmbt_rec_t); - xfs_trans_log_inode(args->trans, dp, XFS_ILOG_CORE); + xfs_attr_fork_reset(dp, args->trans); goto out; } -- cgit v0.10.2 From d4bb6d0698090c485e2e80e8a13852be5a8bfb04 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 4 Feb 2009 09:36:19 +0100 Subject: xfs: merge xfs_inode_flush into xfs_fs_write_inode Splitting the task for a VFS-induced inode flush into two functions doesn't make any sense, so merge the two functions dealing with it. Signed-off-by: Christoph Hellwig Reviewed-by: Felix Blyakher Reviewed-by: Dave Chinner diff --git a/fs/xfs/linux-2.6/xfs_super.c b/fs/xfs/linux-2.6/xfs_super.c index c71e226..faf3aa3 100644 --- a/fs/xfs/linux-2.6/xfs_super.c +++ b/fs/xfs/linux-2.6/xfs_super.c @@ -990,26 +990,57 @@ xfs_fs_write_inode( int sync) { struct xfs_inode *ip = XFS_I(inode); + struct xfs_mount *mp = ip->i_mount; int error = 0; - int flags = 0; xfs_itrace_entry(ip); + + if (XFS_FORCED_SHUTDOWN(mp)) + return XFS_ERROR(EIO); + if (sync) { error = xfs_wait_on_pages(ip, 0, -1); if (error) - goto out_error; - flags |= FLUSH_SYNC; + goto out; + } + + /* + * Bypass inodes which have already been cleaned by + * the inode flush clustering code inside xfs_iflush + */ + if (xfs_inode_clean(ip)) + goto out; + + /* + * We make this non-blocking if the inode is contended, return + * EAGAIN to indicate to the caller that they did not succeed. + * This prevents the flush path from blocking on inodes inside + * another operation right now, they get caught later by xfs_sync. + */ + if (sync) { + xfs_ilock(ip, XFS_ILOCK_SHARED); + xfs_iflock(ip); + + error = xfs_iflush(ip, XFS_IFLUSH_SYNC); + } else { + error = EAGAIN; + if (!xfs_ilock_nowait(ip, XFS_ILOCK_SHARED)) + goto out; + if (xfs_ipincount(ip) || !xfs_iflock_nowait(ip)) + goto out_unlock; + + error = xfs_iflush(ip, XFS_IFLUSH_ASYNC_NOBLOCK); } - error = xfs_inode_flush(ip, flags); -out_error: + out_unlock: + xfs_iunlock(ip, XFS_ILOCK_SHARED); + out: /* * if we failed to write out the inode then mark * it dirty again so we'll try again later. */ if (error) xfs_mark_inode_dirty_sync(ip); - return -error; } diff --git a/fs/xfs/linux-2.6/xfs_vnode.h b/fs/xfs/linux-2.6/xfs_vnode.h index f65983a..ea4675c 100644 --- a/fs/xfs/linux-2.6/xfs_vnode.h +++ b/fs/xfs/linux-2.6/xfs_vnode.h @@ -41,11 +41,6 @@ struct attrlist_cursor_kern; #define IO_INVIS 0x00020 /* don't update inode timestamps */ /* - * Flags for xfs_inode_flush - */ -#define FLUSH_SYNC 1 /* wait for flush to complete */ - -/* * Flush/Invalidate options for vop_toss/flush/flushinval_pages. */ #define FI_NONE 0 /* none */ diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c index 4229408..bc0a0a7 100644 --- a/fs/xfs/xfs_vnodeops.c +++ b/fs/xfs/xfs_vnodeops.c @@ -2589,51 +2589,6 @@ std_return: } int -xfs_inode_flush( - xfs_inode_t *ip, - int flags) -{ - xfs_mount_t *mp = ip->i_mount; - int error = 0; - - if (XFS_FORCED_SHUTDOWN(mp)) - return XFS_ERROR(EIO); - - /* - * Bypass inodes which have already been cleaned by - * the inode flush clustering code inside xfs_iflush - */ - if (xfs_inode_clean(ip)) - return 0; - - /* - * We make this non-blocking if the inode is contended, - * return EAGAIN to indicate to the caller that they - * did not succeed. This prevents the flush path from - * blocking on inodes inside another operation right - * now, they get caught later by xfs_sync. - */ - if (flags & FLUSH_SYNC) { - xfs_ilock(ip, XFS_ILOCK_SHARED); - xfs_iflock(ip); - } else if (xfs_ilock_nowait(ip, XFS_ILOCK_SHARED)) { - if (xfs_ipincount(ip) || !xfs_iflock_nowait(ip)) { - xfs_iunlock(ip, XFS_ILOCK_SHARED); - return EAGAIN; - } - } else { - return EAGAIN; - } - - error = xfs_iflush(ip, (flags & FLUSH_SYNC) ? XFS_IFLUSH_SYNC - : XFS_IFLUSH_ASYNC_NOBLOCK); - xfs_iunlock(ip, XFS_ILOCK_SHARED); - - return error; -} - - -int xfs_set_dmattrs( xfs_inode_t *ip, u_int evmask, diff --git a/fs/xfs/xfs_vnodeops.h b/fs/xfs/xfs_vnodeops.h index 76df328..2258df3 100644 --- a/fs/xfs/xfs_vnodeops.h +++ b/fs/xfs/xfs_vnodeops.h @@ -38,7 +38,6 @@ int xfs_readdir(struct xfs_inode *dp, void *dirent, size_t bufsize, int xfs_symlink(struct xfs_inode *dp, struct xfs_name *link_name, const char *target_path, mode_t mode, struct xfs_inode **ipp, cred_t *credp); -int xfs_inode_flush(struct xfs_inode *ip, int flags); int xfs_set_dmattrs(struct xfs_inode *ip, u_int evmask, u_int16_t state); int xfs_reclaim(struct xfs_inode *ip); int xfs_change_file_space(struct xfs_inode *ip, int cmd, -- cgit v0.10.2 From ef8f7fc549bf345d92f396f5aa7b152b4969cbf7 Mon Sep 17 00:00:00 2001 From: Josef 'Jeff' Sipek Date: Wed, 4 Feb 2009 09:37:43 +0100 Subject: xfs: cleanup error handling in xfs_swap_extents Use multiple lables for proper error unwinding and get rid of some now superflous variables. Signed-off-by: Josef 'Jeff' Sipek Signed-off-by: Christoph Hellwig Tested-by: Christoph Hellwig Reviewed-by: Felix Blyakher diff --git a/fs/xfs/xfs_dfrag.c b/fs/xfs/xfs_dfrag.c index f8278cf..ac96ab9 100644 --- a/fs/xfs/xfs_dfrag.c +++ b/fs/xfs/xfs_dfrag.c @@ -118,19 +118,17 @@ xfs_swap_extents( xfs_bstat_t *sbp = &sxp->sx_stat; xfs_ifork_t *tempifp, *ifp, *tifp; int ilf_fields, tilf_fields; - static uint lock_flags = XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL; int error = 0; int aforkblks = 0; int taforkblks = 0; __uint64_t tmp; - char locked = 0; mp = ip->i_mount; tempifp = kmem_alloc(sizeof(xfs_ifork_t), KM_MAYFAIL); if (!tempifp) { error = XFS_ERROR(ENOMEM); - goto error0; + goto out; } sbp = &sxp->sx_stat; @@ -143,25 +141,24 @@ xfs_swap_extents( */ xfs_lock_two_inodes(ip, tip, XFS_IOLOCK_EXCL); xfs_lock_two_inodes(ip, tip, XFS_ILOCK_EXCL); - locked = 1; /* Verify that both files have the same format */ if ((ip->i_d.di_mode & S_IFMT) != (tip->i_d.di_mode & S_IFMT)) { error = XFS_ERROR(EINVAL); - goto error0; + goto out_unlock; } /* Verify both files are either real-time or non-realtime */ if (XFS_IS_REALTIME_INODE(ip) != XFS_IS_REALTIME_INODE(tip)) { error = XFS_ERROR(EINVAL); - goto error0; + goto out_unlock; } /* Should never get a local format */ if (ip->i_d.di_format == XFS_DINODE_FMT_LOCAL || tip->i_d.di_format == XFS_DINODE_FMT_LOCAL) { error = XFS_ERROR(EINVAL); - goto error0; + goto out_unlock; } if (VN_CACHED(VFS_I(tip)) != 0) { @@ -169,13 +166,13 @@ xfs_swap_extents( error = xfs_flushinval_pages(tip, 0, -1, FI_REMAPF_LOCKED); if (error) - goto error0; + goto out_unlock; } /* Verify O_DIRECT for ftmp */ if (VN_CACHED(VFS_I(tip)) != 0) { error = XFS_ERROR(EINVAL); - goto error0; + goto out_unlock; } /* Verify all data are being swapped */ @@ -183,7 +180,7 @@ xfs_swap_extents( sxp->sx_length != ip->i_d.di_size || sxp->sx_length != tip->i_d.di_size) { error = XFS_ERROR(EFAULT); - goto error0; + goto out_unlock; } /* @@ -193,7 +190,7 @@ xfs_swap_extents( */ if ( XFS_IFORK_Q(ip) != XFS_IFORK_Q(tip) ) { error = XFS_ERROR(EINVAL); - goto error0; + goto out_unlock; } /* @@ -208,7 +205,7 @@ xfs_swap_extents( (sbp->bs_mtime.tv_sec != ip->i_d.di_mtime.t_sec) || (sbp->bs_mtime.tv_nsec != ip->i_d.di_mtime.t_nsec)) { error = XFS_ERROR(EBUSY); - goto error0; + goto out_unlock; } /* We need to fail if the file is memory mapped. Once we have tossed @@ -219,7 +216,7 @@ xfs_swap_extents( */ if (VN_MAPPED(VFS_I(ip))) { error = XFS_ERROR(EBUSY); - goto error0; + goto out_unlock; } xfs_iunlock(ip, XFS_ILOCK_EXCL); @@ -242,8 +239,7 @@ xfs_swap_extents( xfs_iunlock(ip, XFS_IOLOCK_EXCL); xfs_iunlock(tip, XFS_IOLOCK_EXCL); xfs_trans_cancel(tp, 0); - locked = 0; - goto error0; + goto out; } xfs_lock_two_inodes(ip, tip, XFS_ILOCK_EXCL); @@ -253,19 +249,15 @@ xfs_swap_extents( if ( ((XFS_IFORK_Q(ip) != 0) && (ip->i_d.di_anextents > 0)) && (ip->i_d.di_aformat != XFS_DINODE_FMT_LOCAL)) { error = xfs_bmap_count_blocks(tp, ip, XFS_ATTR_FORK, &aforkblks); - if (error) { - xfs_trans_cancel(tp, 0); - goto error0; - } + if (error) + goto out_trans_cancel; } if ( ((XFS_IFORK_Q(tip) != 0) && (tip->i_d.di_anextents > 0)) && (tip->i_d.di_aformat != XFS_DINODE_FMT_LOCAL)) { error = xfs_bmap_count_blocks(tp, tip, XFS_ATTR_FORK, &taforkblks); - if (error) { - xfs_trans_cancel(tp, 0); - goto error0; - } + if (error) + goto out_trans_cancel; } /* @@ -332,10 +324,10 @@ xfs_swap_extents( IHOLD(ip); - xfs_trans_ijoin(tp, ip, lock_flags); + xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL); IHOLD(tip); - xfs_trans_ijoin(tp, tip, lock_flags); + xfs_trans_ijoin(tp, tip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL); xfs_trans_log_inode(tp, ip, ilf_fields); xfs_trans_log_inode(tp, tip, tilf_fields); @@ -344,19 +336,19 @@ xfs_swap_extents( * If this is a synchronous mount, make sure that the * transaction goes to disk before returning to the user. */ - if (mp->m_flags & XFS_MOUNT_WSYNC) { + if (mp->m_flags & XFS_MOUNT_WSYNC) xfs_trans_set_sync(tp); - } error = xfs_trans_commit(tp, XFS_TRANS_SWAPEXT); - locked = 0; - error0: - if (locked) { - xfs_iunlock(ip, lock_flags); - xfs_iunlock(tip, lock_flags); - } - if (tempifp != NULL) - kmem_free(tempifp); +out_unlock: + xfs_iunlock(ip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL); + xfs_iunlock(tip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL); +out: + kmem_free(tempifp); return error; + +out_trans_cancel: + xfs_trans_cancel(tp, 0); + goto out_unlock; } -- cgit v0.10.2 From 4346cdd4647e5eef15817dbfc2c091cac55e33d9 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Sun, 8 Feb 2009 21:51:14 +0100 Subject: xfs: cleanup xfs_find_handle Remove the superflous igrab by keeping a reference on the path/file all the time and clean up various bits of surrounding code. Signed-off-by: Christoph Hellwig Reviewed-by: Felix Blyakher diff --git a/fs/xfs/linux-2.6/xfs_ioctl.c b/fs/xfs/linux-2.6/xfs_ioctl.c index 4bd1123..6f04493 100644 --- a/fs/xfs/linux-2.6/xfs_ioctl.c +++ b/fs/xfs/linux-2.6/xfs_ioctl.c @@ -78,92 +78,74 @@ xfs_find_handle( int hsize; xfs_handle_t handle; struct inode *inode; + struct file *file = NULL; + struct path path; + int error; + struct xfs_inode *ip; - memset((char *)&handle, 0, sizeof(handle)); - - switch (cmd) { - case XFS_IOC_PATH_TO_FSHANDLE: - case XFS_IOC_PATH_TO_HANDLE: { - struct path path; - int error = user_lpath((const char __user *)hreq->path, &path); + if (cmd == XFS_IOC_FD_TO_HANDLE) { + file = fget(hreq->fd); + if (!file) + return -EBADF; + inode = file->f_path.dentry->d_inode; + } else { + error = user_lpath((const char __user *)hreq->path, &path); if (error) return error; - - ASSERT(path.dentry); - ASSERT(path.dentry->d_inode); - inode = igrab(path.dentry->d_inode); - path_put(&path); - break; + inode = path.dentry->d_inode; } + ip = XFS_I(inode); - case XFS_IOC_FD_TO_HANDLE: { - struct file *file; - - file = fget(hreq->fd); - if (!file) - return -EBADF; + /* + * We can only generate handles for inodes residing on a XFS filesystem, + * and only for regular files, directories or symbolic links. + */ + error = -EINVAL; + if (inode->i_sb->s_magic != XFS_SB_MAGIC) + goto out_put; - ASSERT(file->f_path.dentry); - ASSERT(file->f_path.dentry->d_inode); - inode = igrab(file->f_path.dentry->d_inode); - fput(file); - break; - } + error = -EBADF; + if (!S_ISREG(inode->i_mode) && + !S_ISDIR(inode->i_mode) && + !S_ISLNK(inode->i_mode)) + goto out_put; - default: - ASSERT(0); - return -XFS_ERROR(EINVAL); - } - if (inode->i_sb->s_magic != XFS_SB_MAGIC) { - /* we're not in XFS anymore, Toto */ - iput(inode); - return -XFS_ERROR(EINVAL); - } + memcpy(&handle.ha_fsid, ip->i_mount->m_fixedfsid, sizeof(xfs_fsid_t)); - switch (inode->i_mode & S_IFMT) { - case S_IFREG: - case S_IFDIR: - case S_IFLNK: - break; - default: - iput(inode); - return -XFS_ERROR(EBADF); - } - - /* now we can grab the fsid */ - memcpy(&handle.ha_fsid, XFS_I(inode)->i_mount->m_fixedfsid, - sizeof(xfs_fsid_t)); - hsize = sizeof(xfs_fsid_t); - - if (cmd != XFS_IOC_PATH_TO_FSHANDLE) { - xfs_inode_t *ip = XFS_I(inode); + if (cmd == XFS_IOC_PATH_TO_FSHANDLE) { + /* + * This handle only contains an fsid, zero the rest. + */ + memset(&handle.ha_fid, 0, sizeof(handle.ha_fid)); + hsize = sizeof(xfs_fsid_t); + } else { int lock_mode; - /* need to get access to the xfs_inode to read the generation */ lock_mode = xfs_ilock_map_shared(ip); - - /* fill in fid section of handle from inode */ handle.ha_fid.fid_len = sizeof(xfs_fid_t) - sizeof(handle.ha_fid.fid_len); handle.ha_fid.fid_pad = 0; handle.ha_fid.fid_gen = ip->i_d.di_gen; handle.ha_fid.fid_ino = ip->i_ino; - xfs_iunlock_map_shared(ip, lock_mode); hsize = XFS_HSIZE(handle); } - /* now copy our handle into the user buffer & write out the size */ + error = -EFAULT; if (copy_to_user(hreq->ohandle, &handle, hsize) || - copy_to_user(hreq->ohandlen, &hsize, sizeof(__s32))) { - iput(inode); - return -XFS_ERROR(EFAULT); - } + copy_to_user(hreq->ohandlen, &hsize, sizeof(__s32))) + goto out_put; - iput(inode); - return 0; + error = 0; + + out_put: + if (cmd == XFS_IOC_FD_TO_HANDLE) + fput(file); + else + path_put(&path); + return error; } /* -- cgit v0.10.2 From 8e9b6e7fa4544ea8a0e030c8987b918509c8ff47 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Sun, 8 Feb 2009 21:51:42 +0100 Subject: xfs: remove the unused XFS_QMOPT_DQLOCK flag The XFS_QMOPT_DQLOCK flag introduces major complexity in the quota subsystem but isn't actually used anywhere. So remove it and all the hazzles it introduces. Signed-off-by: Christoph Hellwig Reviewed-by: Felix Blyakher diff --git a/fs/xfs/quota/xfs_qm.c b/fs/xfs/quota/xfs_qm.c index 7a2beb6..fd0b383 100644 --- a/fs/xfs/quota/xfs_qm.c +++ b/fs/xfs/quota/xfs_qm.c @@ -632,7 +632,6 @@ xfs_qm_dqattach_one( xfs_dqid_t id, uint type, uint doalloc, - uint dolock, xfs_dquot_t *udqhint, /* hint */ xfs_dquot_t **IO_idqpp) { @@ -641,16 +640,16 @@ xfs_qm_dqattach_one( ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL)); error = 0; + /* * See if we already have it in the inode itself. IO_idqpp is * &i_udquot or &i_gdquot. This made the code look weird, but * made the logic a lot simpler. */ - if ((dqp = *IO_idqpp)) { - if (dolock) - xfs_dqlock(dqp); + dqp = *IO_idqpp; + if (dqp) { xfs_dqtrace_entry(dqp, "DQATTACH: found in ip"); - goto done; + return 0; } /* @@ -659,38 +658,38 @@ xfs_qm_dqattach_one( * lookup by dqid (xfs_qm_dqget) by caching a group dquot inside * the user dquot. */ - ASSERT(!udqhint || type == XFS_DQ_GROUP || type == XFS_DQ_PROJ); - if (udqhint && !dolock) + if (udqhint) { + ASSERT(type == XFS_DQ_GROUP || type == XFS_DQ_PROJ); xfs_dqlock(udqhint); - /* - * No need to take dqlock to look at the id. - * The ID can't change until it gets reclaimed, and it won't - * be reclaimed as long as we have a ref from inode and we hold - * the ilock. - */ - if (udqhint && - (dqp = udqhint->q_gdquot) && - (be32_to_cpu(dqp->q_core.d_id) == id)) { - ASSERT(XFS_DQ_IS_LOCKED(udqhint)); - xfs_dqlock(dqp); - XFS_DQHOLD(dqp); - ASSERT(*IO_idqpp == NULL); - *IO_idqpp = dqp; - if (!dolock) { + /* + * No need to take dqlock to look at the id. + * + * The ID can't change until it gets reclaimed, and it won't + * be reclaimed as long as we have a ref from inode and we + * hold the ilock. + */ + dqp = udqhint->q_gdquot; + if (dqp && be32_to_cpu(dqp->q_core.d_id) == id) { + xfs_dqlock(dqp); + XFS_DQHOLD(dqp); + ASSERT(*IO_idqpp == NULL); + *IO_idqpp = dqp; + xfs_dqunlock(dqp); xfs_dqunlock(udqhint); + return 0; } - goto done; - } - /* - * We can't hold a dquot lock when we call the dqget code. - * We'll deadlock in no time, because of (not conforming to) - * lock ordering - the inodelock comes before any dquot lock, - * and we may drop and reacquire the ilock in xfs_qm_dqget(). - */ - if (udqhint) + + /* + * We can't hold a dquot lock when we call the dqget code. + * We'll deadlock in no time, because of (not conforming to) + * lock ordering - the inodelock comes before any dquot lock, + * and we may drop and reacquire the ilock in xfs_qm_dqget(). + */ xfs_dqunlock(udqhint); + } + /* * Find the dquot from somewhere. This bumps the * reference count of dquot and returns it locked. @@ -698,48 +697,19 @@ xfs_qm_dqattach_one( * disk and we didn't ask it to allocate; * ESRCH if quotas got turned off suddenly. */ - if ((error = xfs_qm_dqget(ip->i_mount, ip, id, type, - doalloc|XFS_QMOPT_DOWARN, &dqp))) { - if (udqhint && dolock) - xfs_dqlock(udqhint); - goto done; - } + error = xfs_qm_dqget(ip->i_mount, ip, id, type, XFS_QMOPT_DOWARN, &dqp); + if (error) + return error; xfs_dqtrace_entry(dqp, "DQATTACH: found by dqget"); + /* * dqget may have dropped and re-acquired the ilock, but it guarantees * that the dquot returned is the one that should go in the inode. */ *IO_idqpp = dqp; - ASSERT(dqp); - ASSERT(XFS_DQ_IS_LOCKED(dqp)); - if (! dolock) { - xfs_dqunlock(dqp); - goto done; - } - if (! udqhint) - goto done; - - ASSERT(udqhint); - ASSERT(dolock); - ASSERT(XFS_DQ_IS_LOCKED(dqp)); - if (! xfs_qm_dqlock_nowait(udqhint)) { - xfs_dqunlock(dqp); - xfs_dqlock(udqhint); - xfs_dqlock(dqp); - } - done: -#ifdef QUOTADEBUG - if (udqhint) { - if (dolock) - ASSERT(XFS_DQ_IS_LOCKED(udqhint)); - } - if (! error) { - if (dolock) - ASSERT(XFS_DQ_IS_LOCKED(dqp)); - } -#endif - return error; + xfs_dqunlock(dqp); + return 0; } @@ -754,24 +724,15 @@ xfs_qm_dqattach_one( STATIC void xfs_qm_dqattach_grouphint( xfs_dquot_t *udq, - xfs_dquot_t *gdq, - uint locked) + xfs_dquot_t *gdq) { xfs_dquot_t *tmp; -#ifdef QUOTADEBUG - if (locked) { - ASSERT(XFS_DQ_IS_LOCKED(udq)); - ASSERT(XFS_DQ_IS_LOCKED(gdq)); - } -#endif - if (! locked) - xfs_dqlock(udq); + xfs_dqlock(udq); if ((tmp = udq->q_gdquot)) { if (tmp == gdq) { - if (! locked) - xfs_dqunlock(udq); + xfs_dqunlock(udq); return; } @@ -781,8 +742,6 @@ xfs_qm_dqattach_grouphint( * because the freelist lock comes before dqlocks. */ xfs_dqunlock(udq); - if (locked) - xfs_dqunlock(gdq); /* * we took a hard reference once upon a time in dqget, * so give it back when the udquot no longer points at it @@ -795,9 +754,7 @@ xfs_qm_dqattach_grouphint( } else { ASSERT(XFS_DQ_IS_LOCKED(udq)); - if (! locked) { - xfs_dqlock(gdq); - } + xfs_dqlock(gdq); } ASSERT(XFS_DQ_IS_LOCKED(udq)); @@ -810,10 +767,9 @@ xfs_qm_dqattach_grouphint( XFS_DQHOLD(gdq); udq->q_gdquot = gdq; } - if (! locked) { - xfs_dqunlock(gdq); - xfs_dqunlock(udq); - } + + xfs_dqunlock(gdq); + xfs_dqunlock(udq); } @@ -821,8 +777,6 @@ xfs_qm_dqattach_grouphint( * Given a locked inode, attach dquot(s) to it, taking U/G/P-QUOTAON * into account. * If XFS_QMOPT_DQALLOC, the dquot(s) will be allocated if needed. - * If XFS_QMOPT_DQLOCK, the dquot(s) will be returned locked. This option pretty - * much made this code a complete mess, but it has been pretty useful. * If XFS_QMOPT_ILOCKED, then inode sent is already locked EXCL. * Inode may get unlocked and relocked in here, and the caller must deal with * the consequences. @@ -851,7 +805,6 @@ xfs_qm_dqattach( if (XFS_IS_UQUOTA_ON(mp)) { error = xfs_qm_dqattach_one(ip, ip->i_d.di_uid, XFS_DQ_USER, flags & XFS_QMOPT_DQALLOC, - flags & XFS_QMOPT_DQLOCK, NULL, &ip->i_udquot); if (error) goto done; @@ -863,11 +816,9 @@ xfs_qm_dqattach( error = XFS_IS_GQUOTA_ON(mp) ? xfs_qm_dqattach_one(ip, ip->i_d.di_gid, XFS_DQ_GROUP, flags & XFS_QMOPT_DQALLOC, - flags & XFS_QMOPT_DQLOCK, ip->i_udquot, &ip->i_gdquot) : xfs_qm_dqattach_one(ip, ip->i_d.di_projid, XFS_DQ_PROJ, flags & XFS_QMOPT_DQALLOC, - flags & XFS_QMOPT_DQLOCK, ip->i_udquot, &ip->i_gdquot); /* * Don't worry about the udquot that we may have @@ -898,22 +849,13 @@ xfs_qm_dqattach( /* * Attach i_gdquot to the gdquot hint inside the i_udquot. */ - xfs_qm_dqattach_grouphint(ip->i_udquot, ip->i_gdquot, - flags & XFS_QMOPT_DQLOCK); + xfs_qm_dqattach_grouphint(ip->i_udquot, ip->i_gdquot); } done: #ifdef QUOTADEBUG if (! error) { - if (ip->i_udquot) { - if (flags & XFS_QMOPT_DQLOCK) - ASSERT(XFS_DQ_IS_LOCKED(ip->i_udquot)); - } - if (ip->i_gdquot) { - if (flags & XFS_QMOPT_DQLOCK) - ASSERT(XFS_DQ_IS_LOCKED(ip->i_gdquot)); - } if (XFS_IS_UQUOTA_ON(mp)) ASSERT(ip->i_udquot); if (XFS_IS_OQUOTA_ON(mp)) diff --git a/fs/xfs/quota/xfs_trans_dquot.c b/fs/xfs/quota/xfs_trans_dquot.c index 9961138..447173b 100644 --- a/fs/xfs/quota/xfs_trans_dquot.c +++ b/fs/xfs/quota/xfs_trans_dquot.c @@ -624,10 +624,9 @@ xfs_trans_dqresv( xfs_qcnt_t *resbcountp; xfs_quotainfo_t *q = mp->m_quotainfo; - if (! (flags & XFS_QMOPT_DQLOCK)) { - xfs_dqlock(dqp); - } - ASSERT(XFS_DQ_IS_LOCKED(dqp)); + + xfs_dqlock(dqp); + if (flags & XFS_TRANS_DQ_RES_BLKS) { hardlimit = be64_to_cpu(dqp->q_core.d_blk_hardlimit); if (!hardlimit) @@ -740,10 +739,8 @@ xfs_trans_dqresv( ASSERT(dqp->q_res_icount >= be64_to_cpu(dqp->q_core.d_icount)); error_return: - if (! (flags & XFS_QMOPT_DQLOCK)) { - xfs_dqunlock(dqp); - } - return (error); + xfs_dqunlock(dqp); + return error; } @@ -753,8 +750,7 @@ error_return: * grp/prj quotas is important, because this follows a both-or-nothing * approach. * - * flags = XFS_QMOPT_DQLOCK indicate if dquot(s) need to be locked. - * XFS_QMOPT_FORCE_RES evades limit enforcement. Used by chown. + * flags = XFS_QMOPT_FORCE_RES evades limit enforcement. Used by chown. * XFS_QMOPT_ENOSPC returns ENOSPC not EDQUOT. Used by pquota. * XFS_TRANS_DQ_RES_BLKS reserves regular disk blocks * XFS_TRANS_DQ_RES_RTBLKS reserves realtime disk blocks diff --git a/fs/xfs/xfs_quota.h b/fs/xfs/xfs_quota.h index 48965ec..63d41ac 100644 --- a/fs/xfs/xfs_quota.h +++ b/fs/xfs/xfs_quota.h @@ -185,7 +185,6 @@ typedef struct xfs_qoff_logformat { * to a single function. None of these XFS_QMOPT_* flags are meant to have * persistent values (ie. their values can and will change between versions) */ -#define XFS_QMOPT_DQLOCK 0x0000001 /* dqlock */ #define XFS_QMOPT_DQALLOC 0x0000002 /* alloc dquot ondisk if needed */ #define XFS_QMOPT_UQUOTA 0x0000004 /* user dquot requested */ #define XFS_QMOPT_PQUOTA 0x0000008 /* project dquot requested */ -- cgit v0.10.2