xfs
[Top] [All Lists]

RE: [PATCH 07/14] repair: use single prefetch queue

To: "Christoph Hellwig" <hch@xxxxxxxxxxxxx>
Subject: RE: [PATCH 07/14] repair: use single prefetch queue
From: "Alex Elder" <aelder@xxxxxxx>
Date: Wed, 21 Oct 2009 12:48:35 -0500
Cc: "Barry Naujok" <bnaujok@xxxxxxx>, <xfs@xxxxxxxxxxx>
In-reply-to: <20090902175840.934778714@xxxxxxxxxxxxxxxxxxxxxx>
Thread-index: Acor+Q0AAq44qVD+TsqwVVqkKuLLXwmfS2mA
Thread-topic: [PATCH 07/14] repair: use single prefetch queue
Christoph Hellwig wrote:
> We don't need two prefetch queues as we guarantee execution in order anyway.
> 
> XXX: description could use some more details.
> 
> 
> Signed-off-by: Barry Naujok <bnaujok@xxxxxxx>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>

One nit-picky comment below, but looks good.

Reviewed-by: Alex Elder <aelder@xxxxxxx>

> Index: xfsprogs-dev/repair/prefetch.c
> ===================================================================
> --- xfsprogs-dev.orig/repair/prefetch.c       2009-08-20 00:14:08.000000000 
> +0000
> +++ xfsprogs-dev/repair/prefetch.c    2009-08-20 00:16:01.000000000 +0000

. . .

The following hunk doesn't really do anything but change whitespace.
It'd be nice if those changes (when there's a bunch like this) were
limited to a separate no-op patch.

> @@ -440,21 +442,22 @@
>                */
>               first_off = LIBXFS_BBTOOFF64(XFS_BUF_ADDR(bplist[0]));
>               last_off = LIBXFS_BBTOOFF64(XFS_BUF_ADDR(bplist[num-1])) +
> -                     XFS_BUF_SIZE(bplist[num-1]);
> +                                             XFS_BUF_SIZE(bplist[num-1]);
>               while (last_off - first_off > pf_max_bytes) {
>                       num--;
> -                     last_off = 
> LIBXFS_BBTOOFF64(XFS_BUF_ADDR(bplist[num-1])) +
> -                             XFS_BUF_SIZE(bplist[num-1]);
> +                     last_off = LIBXFS_BBTOOFF64(XFS_BUF_ADDR(
> +                             bplist[num-1])) + XFS_BUF_SIZE(bplist[num-1]);
>               }
> -             if (num < ((last_off - first_off) >> (mp->m_sb.sb_blocklog + 
> 3))) {
> +             if (num < ((last_off - first_off) >>
> +                                             (mp->m_sb.sb_blocklog + 3))) {
>                       /*
>                        * not enough blocks for one big read, so determine
>                        * the number of blocks that are close enough.
>                        */
>                       last_off = first_off + XFS_BUF_SIZE(bplist[0]);
>                       for (i = 1; i < num; i++) {
> -                             next_off = 
> LIBXFS_BBTOOFF64(XFS_BUF_ADDR(bplist[i])) +
> -                                             XFS_BUF_SIZE(bplist[i]);
> +                             next_off = LIBXFS_BBTOOFF64(XFS_BUF_ADDR(
> +                                     bplist[i])) + XFS_BUF_SIZE(bplist[i]);
>                               if (next_off - last_off > pf_batch_bytes)
>                                       break;
>                               last_off = next_off;

<Prev in Thread] Current Thread [Next in Thread>
  • RE: [PATCH 07/14] repair: use single prefetch queue, Alex Elder <=