Re: [PATCH 1/5] xfs: remove efi from AIL in log recovery

Dave Chinner
2014-07-07T23:44:58Z

On Mon, Jul 07, 2014 at 10:29:02AM -0500, Mark Tinguely wrote:
> On 07/07/14 09:30, Brian Foster wrote:
> >On Wed, Jul 02, 2014 at 09:32:07AM -0500, Mark Tinguely wrote:
> >> (extp->ext_len == 0) ||
> >> (startblock_fsb>= mp->m_sb.sb_dblocks) ||
> >> (extp->ext_len>= mp->m_sb.sb_agblocks)) {
> >>- /*
> >>- * This will pull the EFI from the AIL and
> >>- * free the memory associated with it.
> >>- */
> >>- set_bit(XFS_EFI_RECOVERED,&efip->efi_flags);
> >>- xfs_efi_release(efip, efip->efi_format.efi_nextents);
> >>- return XFS_ERROR(EIO);
> >>+ error = XFS_ERROR(EIO);
> >>+ goto return_free;
> >
> >This bit doesn't apply to for-next. I get similar problems with less
> >trivial hunks on the subsequent patch as well. Looks like you might need
> >to rebase the series onto the recent error negation patches..?
>
>
> WTF? Why are the changes not in the dev tree? Why do people have to
> do development to the for-next tree?

You don't. Brian just asked you to rebase against it because of
merge conflicts against the current for-next tree he tried to apply
it to. It's obvious to me that you didn't read the discussions about
the plans for this dev cycle that went on a month ago:

"[DISCUSS] Planning for new dev cycle (3.17)"

http://oss.sgi.com/pipermail/xfs/2014-June/036739.html

Fact is, this major restructure has been discussed several times
over the past couple of months, both on the mailing list and on IRC,
and there's been plenty of warning and requests for comments about
it.

As to being asked to rebase the patch set on the for-next tree,
I normally do the merges without anybody having to care about the
mismatches due to patch merge order, but we don't normally have a
massive set of changes queued up. Most conflicts are trivial, but
because of the massive change already queued up, the changes this
time are not trivial and so rather than have everyone who wants to
review your code have to mangle them to test with the for-next code,
he's asked you rebase the patchset against for-next.

There's nothing wrong with that - this is a pretty normal thing to
have to do when working with topic branches in the upstream repo. I
was just about to send an email to ask you rebase - lucky I read
this first. ;)

FWIW, you should be doing all your testing against for-next, not
against the master branch. i.e.

$ git checkout -b dev-branch master
<hack hackity hack>
$ git commit
<build, test>
$ git checkout -b testing for-next
$ git merge dev-branch
<build, test, test, test>

Otherwise you aren't testing your changes with all the other changes
that have been committed in the current dev cycle....

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com