On Sun, 2011-01-02 at 02:22 -0500, Christoph Hellwig wrote:
> Allow manual discards from userspace using the FITRIM ioctl. This is not
> intended to be run during normal workloads, as the freepsace btree walks
> can cause large performance degradation.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
This looks good, but I mention three things below.
Correct them as you see fit, either way:
Reviewed-by: Alex Elder <aelder@xxxxxxx>
> ---
>
> V1 -> V2
>
> - added __user annotations as noted by Alex
> - removed non-blocking agf read as noted by Alex
> - update range->len as noted by Alex
>
> This does not implement the by-bno search or lock break suggestions from
> Dave. Given that the 2.6.38 window is about to close those seem a bit
> risky to me. I'll look into these later.
. . .
> xfs_fs_subr.o \
> Index: xfs/fs/xfs/linux-2.6/xfs_discard.c
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ xfs/fs/xfs/linux-2.6/xfs_discard.c 2011-01-02 08:06:15.828014477 +0100
> @@ -0,0 +1,191 @@
> +/*
> + * Copyright (C) 2010 Red Hat, Inc.
Maybe 2011 now...
> + * All Rights Reserved.
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it would be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write the Free Software Foundation,
> + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +#include "xfs.h"
> +#include "xfs_sb.h"
. . .
> +
> +STATIC int
> +xfs_trim_extents(
> + struct xfs_mount *mp,
. . .
> +
> + /*
> + * If the extent is entirely outside of the range we are
> + * supposed to discard skip it. Do not bother to trim
> + * down partially overlapping ranges for now.
> + */
> + if (XFS_AGB_TO_FSB(mp, agno, fbno) + flen < start ||
> + XFS_AGB_TO_FSB(mp, agno, fbno) > start + len) {
^
I think this should be >=
> + trace_xfs_discard_exclude(mp, agno, fbno, flen);
> + goto next_extent;
> + }
. . .
> Index: xfs/fs/xfs/linux-2.6/xfs_discard.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ xfs/fs/xfs/linux-2.6/xfs_discard.h 2011-01-02 07:11:43.693026629 +0100
Do you want to add a boilerplate copyright header here?
> @@ -0,0 +1,8 @@
> +#ifndef XFS_DISCARD_H
> +#define XFS_DISCARD_H 1
> +
> +struct fstrim_range;
> +
> +extern int xfs_ioc_trim(struct xfs_mount *, struct fstrim_range __user *);
> +
> +#endif /* XFS_DISCARD_H */
. . .
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
