From mboxrd@z Thu Jan  1 00:00:00 1970
From: Bob Peterson <rpeterso@redhat.com>
Date: Tue, 17 Sep 2013 13:33:09 -0400 (EDT)
Subject: [Cluster-devel] [GFS2 PATCH 2/3] GFS2: Drop initial reservation
 if too small
In-Reply-To: <1379409897.2722.10.camel@menhir>
References: <3bb02ee14e049c42d6644f003812ca953e186e8e.1379361813.git.rpeterso@redhat.com>
	<4e9fc47e229352b8c2e2b7500df71fb9ac9a4a00.1379361813.git.rpeterso@redhat.com>
	<1379409897.2722.10.camel@menhir>
Message-ID: <1861488338.16777852.1379439189927.JavaMail.root@redhat.com>
List-Id: <cluster-devel.redhat.com>
To: cluster-devel.redhat.com
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit

----- Original Message -----
| Hi,
| 
| On Mon, 2013-09-16 at 15:10 -0500, Bob Peterson wrote:
| > Before this patch, function gfs2_inplace_reserve would grab a blocking
| > lock on the rgrp glock. This is not necessary if we already know that
| > rgrp doesn't have enough blocks to satisfy the request. This patch
| > makes the function drop the inadequate reservation so that a more
| > suitable rgrp may be found (with a try lock). This is not a very
| > important thing to do, and it still requires a blocking lock to remove
| > the reservation. However, it allows us to further simplify the code
| > with a future patch because after this patch, we will know for sure
| > that the reservation will not be in the rgrp's reservations tree
| > unless it has enough blocks.
| > ---
| >  fs/gfs2/rgrp.c | 45 +++++++++++++++++++++++++++++++++++++++++----
| >  1 file changed, 41 insertions(+), 4 deletions(-)
| > 
| This looks like the wrong thing to do. If we have a reservation, then we
| should always use it until it is used up, otherwise we are adding more
| fragmentation when we do not need to. The requested amount is supposed
| to be the number of blocks that we'd like to have in an ideal world, but
| provided we can supply the minimum number of blocks (currently 1) then
| there is no need to drop the reservation.
| 
| Also, I don't want to reintroduce the try locks now that we've
| eliminated them from this part of the code, since that brings back
| another set of problems. Maybe I'm not understanding what the intent is
| here though as despite the comment above, the patches don't appear to
| actually bring the try lock back.
| 
| postmark only tends to write very small files, depending on what
| settings it is being run with, so will likely not gain from the
| reservations work at all. The main reason for using it is to ensure that
| we don't get a regression rather than expecting it to show any
| improvements.
| 
| Steve.

Hi,

Actually, unless I'm reading the code wrong, it already does this.
If this is the wrong thing to do, we need to change the logic.
In today's gfs2_inplace_reserve, there's a check:

		if (rs->rs_rbm.rgd->rd_free_clone >= requested) {
			ip->i_rgd = rs->rs_rbm.rgd;
			return 0;
		}
                (otherwise it gets rejected)

This similarly rejects any pre-existing reservation if it doesn't satisfy
the requested size. What I was trying to do here is preserve the existing
logic. If it's wrong and we need to change it, that's fine, but I have
doubts that it will pass all the testing; when I was working on the
block reservations code, I believe I tried that and got into some nasty
problems. Still, it may be worth trying again.

Regards,

Bob Peterson
Red Hat File Systems