Message ID | 20111202182054.GA3250@arm.com |
---|---|
State | New |
Headers | show |
On Fri, Dec 02, 2011 at 06:20:54PM +0000, Catalin Marinas wrote: > Hi Russell, > > If there are no further comments, could you please pull the ARM LPAE > branch below? They should be merged after Will's idmap patches (no real > conflict, just correctly functioning setup_mm_for_reboot). Err, your tree contains a serious error. tree bb5f2f68dd26a97343d255bb0b66f327cece91a5 parent b4521e369180054ddaacecbe83eadac8878f3501 author Russell King <rmk+kernel@arm.linux.org.uk> 1321983028 +0000 committer Catalin Marinas <catalin.marinas@arm.com> 1322842272 +0000 ARM: pgtable: switch to use pgtable-nopud.h Nick Piggin noted upon introducing 4level-fixup.h: | Add a temporary "fallback" header so architectures can run with | the 4level pagetables patch without modification. All architectures | should be converted to use the folding headers (include/asm-generic/ | pgtable-nop?d.h) as soon as possible, and the fallback header removed. This makes ARM compliant with this statement. Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> If you commit a patch into a git tree, _you_ are supposed to _always_ add your own sign-off line to it unless it's already there. Because of this, I'm going to _have_ to throw out the merge of this from the devel-stable branch (sorry folks) because it's not compliant with the DCO standards required.
On Tue, Dec 06, 2011 at 12:41:09PM +0000, Russell King - ARM Linux wrote: > On Fri, Dec 02, 2011 at 06:20:54PM +0000, Catalin Marinas wrote: > > If there are no further comments, could you please pull the ARM LPAE > > branch below? They should be merged after Will's idmap patches (no real > > conflict, just correctly functioning setup_mm_for_reboot). > > Err, your tree contains a serious error. I would say minor error but I agree, it needs fixing. This patch wasn't originally part of my LPAE series as I hoped you would have merged it during the last cycle. Now it had to be part of the pull request as LPAE patches depend on it. Anyway, I'll fix this and send you another pull request. Unless you already merged your nopud patch in the devel-stable branch and I can rebase LPAE on top, just let me know which one you'd prefer (and a commit id if the latter). Thanks.
On Tue, Dec 06, 2011 at 02:07:29PM +0000, Catalin Marinas wrote: > On Tue, Dec 06, 2011 at 12:41:09PM +0000, Russell King - ARM Linux wrote: > > On Fri, Dec 02, 2011 at 06:20:54PM +0000, Catalin Marinas wrote: > > > If there are no further comments, could you please pull the ARM LPAE > > > branch below? They should be merged after Will's idmap patches (no real > > > conflict, just correctly functioning setup_mm_for_reboot). > > > > Err, your tree contains a serious error. > > I would say minor error but I agree, it needs fixing. The DCO is a viewed by many as having legal implications. Not doing it means - in this case - that you do not believe you have the right to pass the change on. In that case, I can't accept it from you. If you don't understand that, please, go read Documentation/SubmittingPatches to learn about when you are required to sign off on patches before committing them. > This patch wasn't > originally part of my LPAE series as I hoped you would have merged it > during the last cycle. Now it had to be part of the pull request as LPAE > patches depend on it. I've stated many times why it's not merged, and for the N'th time: it generates warnings. I'm _not_ merging something that is known to add warnings such as those which this patch produces without there being a fix for it. You know that _very well_ because I've said it several times, not only by email but also on our various phone calls. I've dealt with this patch in exactly the same way at every merge window we've had for the last _year_ - I've queued it up with the expectation that hopefully someone would fix the warnings, the warnings didn't get fixed, so it got dropped from the pull request. Immediately after the merge window (which includes this) it gets reinstated back into linux-next. > P.S. I say it's minor because it has the committer information. If you > would pull someone else's branch which results in a fast-forward, > you can't really tell whether it was a merge or cherry-pick, so the > rule is not entirely consistent (unless you also mandate --no-ff > for merges). That is a known and accepted "limitation" - as is the "limitation" that you can't add the sign-off to each commit you've pulled in. What Linus really wants is stuff to be tracked to the point it enters into git via sign-offs. From then on, the accountability trail follows the merge commits.
On Tue, Dec 06, 2011 at 11:30:58PM +0000, Russell King - ARM Linux wrote: > On Tue, Dec 06, 2011 at 02:07:29PM +0000, Catalin Marinas wrote: > > This patch wasn't > > originally part of my LPAE series as I hoped you would have merged it > > during the last cycle. Now it had to be part of the pull request as LPAE > > patches depend on it. > > I've stated many times why it's not merged, and for the N'th time: it > generates warnings. I'm _not_ merging something that is known to add > warnings such as those which this patch produces without there being a > fix for it. You know that _very well_ because I've said it several > times, not only by email but also on our various phone calls. > > I've dealt with this patch in exactly the same way at every merge window > we've had for the last _year_ - I've queued it up with the expectation > that hopefully someone would fix the warnings, the warnings didn't get > fixed, so it got dropped from the pull request. Immediately after the > merge window (which includes this) it gets reinstated back into > linux-next. Yes, I'm fully aware, and I sent you a fix-up in the past. I can re-write that fix-up in a few other ways if you don't like the current one, just let me know.
On Wed, Dec 07, 2011 at 11:23:20AM +0000, Catalin Marinas wrote: > On Tue, Dec 06, 2011 at 11:30:58PM +0000, Russell King - ARM Linux wrote: > > On Tue, Dec 06, 2011 at 02:07:29PM +0000, Catalin Marinas wrote: > > > This patch wasn't > > > originally part of my LPAE series as I hoped you would have merged it > > > during the last cycle. Now it had to be part of the pull request as LPAE > > > patches depend on it. > > > > I've stated many times why it's not merged, and for the N'th time: it > > generates warnings. I'm _not_ merging something that is known to add > > warnings such as those which this patch produces without there being a > > fix for it. You know that _very well_ because I've said it several > > times, not only by email but also on our various phone calls. > > > > I've dealt with this patch in exactly the same way at every merge window > > we've had for the last _year_ - I've queued it up with the expectation > > that hopefully someone would fix the warnings, the warnings didn't get > > fixed, so it got dropped from the pull request. Immediately after the > > merge window (which includes this) it gets reinstated back into > > linux-next. > > Yes, I'm fully aware, and I sent you a fix-up in the past. I can > re-write that fix-up in a few other ways if you don't like the current > one, just let me know. So you mean you're submitting me my patch that I've been avoiding merging into mainline _without_ the fixup patch? It seems not - you do have it in your pull request: ARM: pgtable: Fix compiler warning in ioremap.c introduced by nopud So what's the issue? Why do you want to rewrite it yet again? My guess is that you haven't actually READ what I said above (which is quoted). And what is quoted above is _me_ explaining _why_ I have not merged my own patch and why _I_ _consider_ _the_ _patch_ _as_ _it_ _stands_ _to_ _not_ _be_ _immediately_ _suitable_ _for_ _merging_ ___on___ ___its___ ___own___. The fact is, with the "fix" patch, it _does_ become suitable for merging. Are you getting the message yet? Provided you've fixed the sign-off (which you say you have) the only remaining problem is the conflict between your tree and Will's tree which you actually require for LPAE to be buildable. Which, I'll point out, I'm not looking at at the moment because I'm writing this email to you this evening instead.
On Wed, Dec 07, 2011 at 12:25:44PM +0000, Catalin Marinas wrote: > ARM: pgtable: Fix compiler warning in ioremap.c introduced by nopud > > From: Catalin Marinas <catalin.marinas@arm.com> > > With the arch/arm code conversion to pgtable-nopud.h, the section and > supersection (un|re)map code triggers compiler warnings on UP systems. > This is caused by pmd_offset() being given a pgd_t argument rather than > a pud_t one. This patch makes the necessary conversion via pud_alloc() > and pmd_alloc(). > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> So, do I now take this email as you saying that _your_ LPAE tree is not ready for merging because you want to rework this patch, or what? Because you've decided to go down the path of reworking this patch, I consider it dangerous for me to just pull from your tree again - I've no idea what I'd get, and I've no idea whether what's there is what you want me to merge. So, this is causing extra delay, and there's nothing _I_ can do about it at my end. This is getting completely out of hand. What I want from you - by 5pm tomorrow so I can get this stuff merged - is a pull request for the LPAE stuff as it was in your v2 pull request, either based on top of _either_ my devel-stable (if you can get it - as the machine is having severe problems running git atm) or with Will's idmap changes merged into it _before_ the LPAE changes. I suspect you'll have to do the latter though, because I don't think I can sort out git on the server at the moment.
On 7 December 2011 20:29, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Wed, Dec 07, 2011 at 12:25:44PM +0000, Catalin Marinas wrote: >> ARM: pgtable: Fix compiler warning in ioremap.c introduced by nopud >> >> From: Catalin Marinas <catalin.marinas@arm.com> >> >> With the arch/arm code conversion to pgtable-nopud.h, the section and >> supersection (un|re)map code triggers compiler warnings on UP systems. >> This is caused by pmd_offset() being given a pgd_t argument rather than >> a pud_t one. This patch makes the necessary conversion via pud_alloc() >> and pmd_alloc(). >> >> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> > > So, do I now take this email as you saying that _your_ LPAE tree is > not ready for merging because you want to rework this patch, or what? No, I'm not, it's you saying that your nopud patch does not have any fix for the warnings, even though I had fixes posted for months. The same fix that's in my LPAE series was sent to you to include with your patch but instead you kept saying (even twice in the past week) that those warning had no fix. From that, I understand that you really don't consider my patch a proper fix for the compiler warnings. > Because you've decided to go down the path of reworking this patch, > I consider it dangerous for me to just pull from your tree again - I've > no idea what I'd get, and I've no idea whether what's there is what you > want me to merge. I haven't changed anything in my LPAE branch, just posted an alternative fix-up for the nopud patch. Please _state_ clearly whether you consider my original (in the LPAE series) fixup valid or not. If yes, I'm happy that I don't have to rework anything. > What I want from you - by 5pm tomorrow so I can get this stuff merged - > is a pull request for the LPAE stuff as it was in your v2 pull request, > either based on top of _either_ my devel-stable (if you can get it - as > the machine is having severe problems running git atm) or with Will's > idmap changes merged into it _before_ the LPAE changes. > > I suspect you'll have to do the latter though, because I don't think I > can sort out git on the server at the moment. Yes, that's fine with me, I already talked to Will about this earlier today but wanted to get your reply first.
On Wed, Dec 07, 2011 at 10:15:24PM +0000, Catalin Marinas wrote: > On 7 December 2011 20:29, Russell King - ARM Linux > <linux@arm.linux.org.uk> wrote: > > On Wed, Dec 07, 2011 at 12:25:44PM +0000, Catalin Marinas wrote: > >> ARM: pgtable: Fix compiler warning in ioremap.c introduced by nopud > >> > >> From: Catalin Marinas <catalin.marinas@arm.com> > >> > >> With the arch/arm code conversion to pgtable-nopud.h, the section and > >> supersection (un|re)map code triggers compiler warnings on UP systems. > >> This is caused by pmd_offset() being given a pgd_t argument rather than > >> a pud_t one. This patch makes the necessary conversion via pud_alloc() > >> and pmd_alloc(). > >> > >> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> > > > > So, do I now take this email as you saying that _your_ LPAE tree is > > not ready for merging because you want to rework this patch, or what? > > No, I'm not, it's you saying that your nopud patch does not have any > fix for the warnings, even though I had fixes posted for months. Stop it Catalin. You're being utterly rediculous. You asked why I hadn't merged my patch during the last merge window. So I told you - just like I've been telling you by email and during our monthly calls why I've not been merging it. You asked the question (again) so I told you why (again). > The > same fix that's in my LPAE series was sent to you to include with your > patch but instead you kept saying (even twice in the past week) that > those warning had no fix. Utter rubbish. I'll say it again. You asked why I hadn't merged my patch. So I told you - just like I've been telling you by email and during our monthly calls why I've not been merging it. You asked the question (again) so I told you why. > From that, I understand that you really > don't consider my patch a proper fix for the compiler warnings. WTF. > > Because you've decided to go down the path of reworking this patch, > > I consider it dangerous for me to just pull from your tree again - I've > > no idea what I'd get, and I've no idea whether what's there is what you > > want me to merge. > > I haven't changed anything in my LPAE branch, just posted an > alternative fix-up for the nopud patch. Please _state_ clearly whether > you consider my original (in the LPAE series) fixup valid or not. If > yes, I'm happy that I don't have to rework anything. Oh for god sake. I think I've already stated very clearly what I want. I see no reason to repeat myself yet again.
On Wed, Dec 07, 2011 at 08:29:12PM +0000, Russell King - ARM Linux wrote: > What I want from you - by 5pm tomorrow so I can get this stuff merged - > is a pull request for the LPAE stuff as it was in your v2 pull request, > either based on top of _either_ my devel-stable (if you can get it - as > the machine is having severe problems running git atm) or with Will's > idmap changes merged into it _before_ the LPAE changes. For the avoidance of any doubt - and any argument - the 5pm time limit is there to bring this debacle to a close in a reasonable period. It's purely there because I want to get this merged by end of Thursday and no later. To summarise, my objections are quite simply: 1. Your lack of sign-off on my patch which you merged (which you've addressed in pull v2.) 2. The combined point of the merge conflict between your tree and Will's idmap tree, plus the dependency your tree _does_ have on the idmap changes means it _should_ include Will's idmap changes to ensure bisectability - and this has the side effect of solving the merge conflict which shouldn't be there. Any other objections which you think I've made are a purely a figment of your imagination. Now, one thing I haven't checked is the ordering of the commits - particularly the ordering of the 'fix' patch and my patch. Unfortunately, standard git pull messages don't give the order so it's not something I can readily check. I commented on this issue when you posted the patch set. I hope it will be correct for pull v3 (iow, the fix patch first, then my patch.)