From mboxrd@z Thu Jan 1 00:00:00 1970 From: Abhijith Das Date: Tue, 17 Feb 2015 10:07:29 -0500 (EST) Subject: [Cluster-devel] [GFS2 PATCH 3/3] gfs2: allow fallocate to max out quotas/fs efficiently In-Reply-To: <54E30CBB.2050401@redhat.com> References: <1424109583-42670-1-git-send-email-adas@redhat.com> <1424109583-42670-4-git-send-email-adas@redhat.com> <54E30CBB.2050401@redhat.com> Message-ID: <1081828134.14360074.1424185649990.JavaMail.zimbra@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit ----- Original Message ----- > From: "Steven Whitehouse" > To: "Abhi Das" , cluster-devel at redhat.com > Sent: Tuesday, February 17, 2015 3:41:15 AM > Subject: Re: [Cluster-devel] [GFS2 PATCH 3/3] gfs2: allow fallocate to max out quotas/fs efficiently > > Hi, > > On 16/02/15 17:59, Abhi Das wrote: > > We can quickly get an estimate of how many more blocks are > > available for allocation restricted by quota and fs size > > respectively using the ap->allowed field in the gfs2_alloc_parms > > structure. gfs2_quota_check() and gfs2_inplace_reserve() provide > > these values. > > > > By re-trying to allocate what's available instead of guessing, we > > can max out quotas or the filesystem efficiently. > > > > Bear in mind that this applies only when the requested fallocate > > operation would otherwise error out with -EDQUOT or -ENOSPC without > > utilizing all the blocks that might still be available. > > > > Signed-off-by: Abhi Das > > --- > > fs/gfs2/file.c | 18 +++++++++++------- > > 1 file changed, 11 insertions(+), 7 deletions(-) > > > > diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c > > index 2ea420a..57129fa 100644 > > --- a/fs/gfs2/file.c > > +++ b/fs/gfs2/file.c > > @@ -829,20 +829,24 @@ static long __gfs2_fallocate(struct file *file, int > > mode, loff_t offset, loff_t > > continue; > > } > > ap.target = bytes >> sdp->sd_sb.sb_bsize_shift; > > + quota_retry: > > error = gfs2_quota_lock_check(ip, &ap); > > - if (error) > > + if (error) { > > + if (error == -EDQUOT && ap.allowed) { > > + bytes = ap.allowed << sdp->sd_sb.sb_bsize_shift; > > + ap.target = ap.allowed; > > + goto quota_retry; > > + } > > return error; > Do you really need to loop here if -EDQUOT is returned and ap.allowed is > set? Why not just continue after having updated the target value? > > Otherwise I think these patches look good, > > Steve. > If gfs2_quota_lock_check() fails, the quota isn't locked, which we'd have to do anyway. And I figured there's a race here where some other process could use up all or part of ap.allowed blocks before we re-lock the quota thereby causing us to exceed quotas. So, I decided to retry gfs2_quota_lock_check() itself. Cheers! --Abhi