| To: | Dave Chinner <david@xxxxxxxxxxxxx> |
|---|---|
| Subject: | Re: [PATCH 05/11] xfs: optimise xfs_iext_destroy |
| From: | Christoph Hellwig <hch@xxxxxxxxxxxxx> |
| Date: | Wed, 13 Apr 2016 09:45:11 -0700 |
| Cc: | xfs@xxxxxxxxxxx |
| Delivered-to: | xfs@xxxxxxxxxxx |
| In-reply-to: | <1460525492-1170-6-git-send-email-david@xxxxxxxxxxxxx> |
| References: | <1460525492-1170-1-git-send-email-david@xxxxxxxxxxxxx> <1460525492-1170-6-git-send-email-david@xxxxxxxxxxxxx> |
| User-agent: | Mutt/1.5.24 (2015-08-30) |
> /*
> + * Remove all records from the indirection array.
> + */
> +STATIC void
> +xfs_iext_irec_remove_all(
> + struct xfs_ifork *ifp)
> +{
> + int nlists;
> + int i;
> +
> + ASSERT(ifp->if_flags & XFS_IFEXTIREC);
> + nlists = ifp->if_real_bytes / XFS_IEXT_BUFSZ;
> + for (i = 0; i < nlists; i++) {
> + xfs_ext_irec_t *erp = &ifp->if_u1.if_ext_irec[i];
Can we avoid the typedef for new code?
> + if (erp->er_extbuf)
> + kmem_free(erp->er_extbuf);
no need to check for NULL before a kmem_free, e.g. this whole loop could
just be:
for (i = 0; i < nlists; i++)
kmem_free(ifp->if_u1.if_ext_irec[i].er_extbuf);
> {
> if (ifp->if_flags & XFS_IFEXTIREC) {
> - int erp_idx;
> - int nlists;
> -
> - nlists = ifp->if_real_bytes / XFS_IEXT_BUFSZ;
> - for (erp_idx = nlists - 1; erp_idx >= 0 ; erp_idx--) {
> - xfs_iext_irec_remove(ifp, erp_idx);
> - }
> + xfs_iext_irec_remove_all(ifp);
> ifp->if_flags &= ~XFS_IFEXTIREC;
I'd be tempted to just move clearing of the flag into
xfs_iext_irec_remove_all if we change the patch anyway.
Otherwise this looks fine to me.
|
| Previous by Date: | Re: [PATCH 02/11] xfs: xfs_iflush_cluster fails to abort on error, Christoph Hellwig |
|---|---|
| Next by Date: | Re: list_add corruption after "xfs: mode di_mode to vfs inode", Joe Lawrence |
| Previous by Thread: | [PATCH 05/11] xfs: optimise xfs_iext_destroy, Dave Chinner |
| Next by Thread: | [PATCH 01/11] xfs: we don't need no steekin ->evict_inode, Dave Chinner |
| Indexes: | [Date] [Thread] [Top] [All Lists] |