headers
Christoph Hellwig | 1 Dec 2011 08:21
Favicon

Re: [PATCH 4/4] xfs: fix the logspace waiting algorithm

On Wed, Nov 30, 2011 at 05:56:41PM -0600, Ben Myers wrote:
> Christoph,
> 
> I feel that the ordering of the accesses to l_grant_head and the writeq
> list may be important in the lockless path, and notice that they used to
> be in opposite order.  It could use a comment explaining why (if) it is
> ok.

Do you mean moving the xlog_space_left after the first
list_empty_careful check in xlog_grant_log_space?  It doesn't matter
given that we re-check the available space after each wakeup.

_______________________________________________
xfs mailing list
xfs <at> oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
Permalink | Reply | 4 comments |
headers
Christoph Hellwig | 1 Dec 2011 10:37
Favicon

Re: [PATCH 5/5] xfs: log file size updates at I/O completion time

Here is a version that uses the transaction reservations from
->writepage context.  Messing with the task flags is a bit ugly, but
it's been surviving QA for a couple of days nows.  Note that this is
against my current pending tree, and won't apply against the series
I sent out before.  I'll send out the rest of my queue soon.

---
From: Christoph Hellwig <hch <at> lst.de>
Subject: xfs: log file size updates at I/O completion time

Do not use unlogged metadata updates and the VFS dirty bit for updating
the file size after writeback.  In addition to causing various problems
with updates getting delayed for far too log this also drags in the
unscalable VFS dirty tracking, and is one of the few remaining unlogged
metadata updates.

Signed-off-by: Christoph Hellwig <hch <at> lst.de>

---
 fs/xfs/xfs_aops.c |  124 ++++++++++++++++++++++++++++++++++++++++--------------
 fs/xfs/xfs_aops.h |    2 
 2 files changed, 95 insertions(+), 31 deletions(-)

Index: xfs/fs/xfs/xfs_aops.c
===================================================================
--- xfs.orig/fs/xfs/xfs_aops.c	2011-11-30 12:54:05.748022545 +0100
+++ xfs/fs/xfs/xfs_aops.c	2011-11-30 12:54:59.891062561 +0100
 <at>  <at>  -26,6 +26,7  <at>  <at> 
 #include "xfs_bmap_btree.h"
 #include "xfs_dinode.h"
(Continue reading)

Permalink | Reply | 0 comments |
headers
Dave Chinner | 1 Dec 2011 12:24

[PATCH] xfs: fix allocation length overflow in xfs_bmapi_write()

From: Dave Chinner <dchinner <at> redhat.com>

When testing the new xfstests --large-fs option that does very large
file preallocations, this assert was tripped deep in
xfs_alloc_vextent():

XFS: Assertion failed: args->minlen <= args->maxlen, file: fs/xfs/xfs_alloc.c, line: 2239

The allocation was trying to allocate a zero length extent because
the lower 32 bits of the allocation length was zero. The remaining
length of the allocation to be done was an exact multiple of 2^32 -
the first case I saw was at 496TB remaining to be allocated.

This turns out to be an overflow when converting the allocation
length (a 64 bit quantity) into the extent length to allocate (a 32
bit quantity), and it requires the length to be allocated an exact
multiple of 2^32 blocks to trip the assert.

Fix it by limiting the extent lenth to allocate to MAXEXTLEN.

Signed-off-by: Dave Chinner <dchinner <at> redhat.com>
---
 fs/xfs/xfs_bmap.c |   20 +++++++++++++++++++-
 1 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/fs/xfs/xfs_bmap.c b/fs/xfs/xfs_bmap.c
index c68baeb..8d93823 100644
--- a/fs/xfs/xfs_bmap.c
+++ b/fs/xfs/xfs_bmap.c
 <at>  <at>  -2383,6 +2383,8  <at>  <at>  xfs_bmap_btalloc(
(Continue reading)

Permalink | Reply | 2 comments |
headers
Carlos Maiolino | 1 Dec 2011 13:44
Picon
Favicon

[PATCH] mkfs: Refuse to initialize a misaligned device if not forced using libblkid

This is the first patch proposal to fix the problem about the usage of
4k sector devices when the device is not properly aligned

The patch make mkfs to refuse to initialize a xfs filesystem if the -f
option is not passed at the command line, and forces a 512b sector size
if the user opt to force the device initialization

Signed-off-by: Carlos Maiolino <cmaiolino <at> redhat.com>
---
 mkfs/xfs_mkfs.c |   24 ++++++++++++++++++------
 1 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index f527f3d..495c70d 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
 <at>  <at>  -369,8 +369,14  <at>  <at>  out:
 	return ret;
 }

-static void blkid_get_topology(const char *device, int *sunit, int *swidth, int *sectorsize)
+static void blkid_get_topology(
+	const char *device,
+	int *sunit,
+	int *swidth,
+	int *sectorsize,
+	int force_overwrite)
 {
+
 	blkid_topology tp;
(Continue reading)

Permalink | Reply | 1 comment |
headers
Ben Myers | 1 Dec 2011 19:32
Picon
Favicon

Re: [PATCH 4/4] xfs: fix the logspace waiting algorithm

Hi,

On Mon, Nov 28, 2011 at 03:17:36AM -0500, Christoph Hellwig wrote:
> Apply the scheme used in log_regrant_write_log_space to wake up any other
> threads waiting for log space before the newly added one to
> log_regrant_write_log_space as well, and factor the code into readable
> helpers.  For each of the queues we have add two helpers:
> 
>  - one to try to wake up all waiting threads.  This helper will also be
>    usable by xfs_log_move_tail once we remove the current opportunistic
>    wakeups in it.
>  - one to sleep on t_wait until enough log space is available, loosely
>    modelled after Linux waitqueues.
>  
> And use them to reimplement the guts of log_regrant_write_log_space and
> log_regrant_write_log_space.  These two function now use one and the same
> algorithm for waiting on log space instead of subtly different ones before,
> with an option to completely unify them in the near future.
> 
> Also move the filesystem shutdown handling to the common caller given
> that we had to touch it anyway.
> 
> Based on hard debugging and an earlier patch from
> Chandra Seetharaman <sekharan <at> us.ibm.com>.
> 
> Signed-off-by: Christoph Hellwig <hch <at> lst.de>

I'd like to make sure that I understand the race that Chandra
debugged and reported.
(Continue reading)

Permalink | Reply | 1 comment |
headers
Chandra Seetharaman | 1 Dec 2011 20:28
Picon
Favicon

Re: [PATCH 4/4] xfs: fix the logspace waiting algorithm

Tested the patch with testcases 234 and 273. They ran for more than 350
iterations without getting into the hang situation.

Tested-by: Chandra Seetharaman <sekharan <at> us.ibm.com>

Few generic comments on the patch

1. xlog_*_wake could use something to indicate that they are looking for
log space in the specific queue. ex: xlog_reserveq_available()

2. All new functions expect a lock to be held on entry. Can be
explicitly specified in a comment.

3. Change the trace function names to reflect the names of the
function ?!

Otherwise patch looks good and works fine.

Chandra
------- end of comments ------
On Mon, 2011-11-28 at 03:17 -0500, Christoph Hellwig wrote:
> plain text document attachment (xfs-fix-log-race)
> Apply the scheme used in log_regrant_write_log_space to wake up any other
> threads waiting for log space before the newly added one to
> log_regrant_write_log_space as well, and factor the code into readable
> helpers.  For each of the queues we have add two helpers:
> 
>  - one to try to wake up all waiting threads.  This helper will also be
>    usable by xfs_log_move_tail once we remove the current opportunistic
>    wakeups in it.
(Continue reading)

Permalink | Reply | 2 comments |
headers
Ben Myers | 1 Dec 2011 20:51
Picon
Favicon

Re: [PATCH 4/4] xfs: fix the logspace waiting algorithm

On Thu, Dec 01, 2011 at 02:21:49AM -0500, Christoph Hellwig wrote:
> On Wed, Nov 30, 2011 at 05:56:41PM -0600, Ben Myers wrote:
> > Christoph,
> > 
> > I feel that the ordering of the accesses to l_grant_head and the writeq
> > list may be important in the lockless path, and notice that they used to
> > be in opposite order.  It could use a comment explaining why (if) it is
> > ok.
> 
> Do you mean moving the xlog_space_left after the first
> list_empty_careful check in xlog_grant_log_space?

That's what I mean, but I'm not sure I was correct.

> It doesn't matter
> given that we re-check the available space after each wakeup.

2620 STATIC int
2621 xlog_grant_log_space(
2622         struct log              *log,
2623         struct xlog_ticket      *tic)
2624 {
2625         int                     free_bytes, need_bytes;
2626         int                     error = 0;
2627 
2628         ASSERT(!(log->l_flags & XLOG_ACTIVE_RECOVERY));
2629 
2630         trace_xfs_log_grant_enter(log, tic);
2631 
2632         /*
(Continue reading)

Permalink | Reply | 3 comments |
headers
Chandra Seetharaman | 1 Dec 2011 21:43
Picon
Favicon

Re: [PATCH 4/4] xfs: fix the logspace waiting algorithm

Ben,

Yes, that is the race I found thru debugging.

Chandra
On Thu, 2011-12-01 at 12:32 -0600, Ben Myers wrote:
> Hi,
> 
> On Mon, Nov 28, 2011 at 03:17:36AM -0500, Christoph Hellwig wrote:
> > Apply the scheme used in log_regrant_write_log_space to wake up any other
> > threads waiting for log space before the newly added one to
> > log_regrant_write_log_space as well, and factor the code into readable
> > helpers.  For each of the queues we have add two helpers:
> > 
> >  - one to try to wake up all waiting threads.  This helper will also be
> >    usable by xfs_log_move_tail once we remove the current opportunistic
> >    wakeups in it.
> >  - one to sleep on t_wait until enough log space is available, loosely
> >    modelled after Linux waitqueues.
> >  
> > And use them to reimplement the guts of log_regrant_write_log_space and
> > log_regrant_write_log_space.  These two function now use one and the same
> > algorithm for waiting on log space instead of subtly different ones before,
> > with an option to completely unify them in the near future.
> > 
> > Also move the filesystem shutdown handling to the common caller given
> > that we had to touch it anyway.
> > 
> > Based on hard debugging and an earlier patch from
> > Chandra Seetharaman <sekharan <at> us.ibm.com>.
(Continue reading)

Permalink | Reply | 0 comments |
headers
Ben Myers | 1 Dec 2011 23:00
Picon
Favicon

Re: [PATCH] xfs: fix allocation length overflow in xfs_bmapi_write()

On Thu, Dec 01, 2011 at 10:24:20PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner <at> redhat.com>
> 
> When testing the new xfstests --large-fs option that does very large
> file preallocations, this assert was tripped deep in
> xfs_alloc_vextent():
> 
> XFS: Assertion failed: args->minlen <= args->maxlen, file: fs/xfs/xfs_alloc.c, line: 2239
> 
> The allocation was trying to allocate a zero length extent because
> the lower 32 bits of the allocation length was zero. The remaining
> length of the allocation to be done was an exact multiple of 2^32 -
> the first case I saw was at 496TB remaining to be allocated.
> 
> This turns out to be an overflow when converting the allocation
> length (a 64 bit quantity) into the extent length to allocate (a 32
> bit quantity), and it requires the length to be allocated an exact
> multiple of 2^32 blocks to trip the assert.
> 
> Fix it by limiting the extent lenth to allocate to MAXEXTLEN.
> 
> Signed-off-by: Dave Chinner <dchinner <at> redhat.com>

Looks good to me.
Reviewed-by: Ben Myers <bpm <at> sgi.com>

_______________________________________________
xfs mailing list
xfs <at> oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
(Continue reading)

Permalink | Reply | 0 comments |
headers
Greg KH | 2 Dec 2011 00:33
Gravatar

Re: [PATCH 0/6] XFS update for 3.1-stable (again)

On Thu, Dec 01, 2011 at 05:27:38PM -0600, Ben Myers wrote:
> This is a series of XFS fixes from current mainline which is important for
> 3.1-stable.  Note that it is the same patch set that Christoph submitted for
> 3.0-stable, minus the first three patches which are already included in 3.1.
> 
> My QA came out ok with these six patches atop 3.1.y.  I've been having trouble
> getting mail out to stable <at> vger, but it seems to be working now.  Apologies to
> those who are getting this mail yet again.

Nice, I got all of these, thanks.  I'll queue them up in a few days when
I get caught up on the stable patch queue.

greg k-h

_______________________________________________
xfs mailing list
xfs <at> oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
Permalink | Reply | 2 comments |

Gmane