[Top] [All Lists]

Re: [PATCH v4 1/6] xfs: reorganize xfs_iomap_prealloc_size to remove ind

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [PATCH v4 1/6] xfs: reorganize xfs_iomap_prealloc_size to remove indentation
From: Mark Tinguely <tinguely@xxxxxxx>
Date: Fri, 22 Feb 2013 12:22:52 -0600
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <5127B438.8080707@xxxxxxxxxx>
References: <1361373019-30891-1-git-send-email-bfoster@xxxxxxxxxx> <1361373019-30891-2-git-send-email-bfoster@xxxxxxxxxx> <5127A5E2.2070407@xxxxxxx> <5127B438.8080707@xxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:9.0) Gecko/20120122 Thunderbird/9.0
On 02/22/13 12:08, Brian Foster wrote:
On 02/22/2013 12:07 PM, Mark Tinguely wrote:
On 02/20/13 09:10, Brian Foster wrote:
The majority of xfs_iomap_prealloc_size() executes within the
check for lack of default I/O size. Reverse the logic to remove the
extra indentation.

Signed-off-by: Brian Foster<bfoster@xxxxxxxxxx>
Reviewed-by: Dave Chinner<dchinner@xxxxxxxxxx>
Reviewed-by: Ben Myers<bpm@xxxxxxx>
   fs/xfs/xfs_iomap.c |   63
   1 files changed, 32 insertions(+), 31 deletions(-)

diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 912d83d..d914419 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -381,42 +381,43 @@ xfs_iomap_prealloc_size(
       int            nimaps)
       xfs_fsblock_t        alloc_blocks = 0;
+    int            shift = 0;
+    int64_t            freesp;


+    freesp = mp->m_sb.sb_fdblocks;


+    /*
+     * If we are still trying to allocate more space than is
+     * available, squash the prealloc hard. This can happen if we
+     * have a large file on a small filesystem and the above
+     * lowspace thresholds are smaller than MAXEXTLEN.
+     */
+    while (alloc_blocks>= freesp)
+        alloc_blocks>>= 4;

Hi Brian, I am looking at your speculative preallocation quota
throttling series.

I know this code is from commit 4d559a3b. would this not be bad of
freesp == 0?

Thanks. Hmm, I guess if freesp is 0 we'd hit an infinite loop
(irrespective of this patchset). We could change the comparison to>,
but I think the following would be more clear:

        while (alloc_blocks&&  alloc_blocks>= freesp)
                alloc_blocks>>= 4;

Thoughts? I'll send out a one-liner to bat around if that looks
reasonable. Good catch.



That looks reasonable to me.



<Prev in Thread] Current Thread [Next in Thread>