| Message ID | 1348874411-28288-7-git-send-email-daniel.santos@pobox.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Fri, Sep 28, 2012 at 06:20:07PM -0500, Daniel Santos wrote: > Signed-off-by: Daniel Santos <daniel.santos@pobox.com> > --- > include/linux/bug.h | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/include/linux/bug.h b/include/linux/bug.h > index aaac4bb..298a916 100644 > --- a/include/linux/bug.h > +++ b/include/linux/bug.h > @@ -73,7 +73,7 @@ extern int __build_bug_on_failed; > #define BUILD_BUG() \ > do { \ > extern void __build_bug_failed(void) \ > - __linktime_error("BUILD_BUG failed"); \ > + __compiletime_error("BUILD_BUG failed");\ > __build_bug_failed(); \ > } while (0) This change should either occur as part of patch 5 or before patch 5, not after. - Josh Triplett -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2012-09-28 at 17:23 -0700, Josh Triplett wrote: > On Fri, Sep 28, 2012 at 06:20:07PM -0500, Daniel Santos wrote: > > Signed-off-by: Daniel Santos <daniel.santos@pobox.com> > > --- > > include/linux/bug.h | 2 +- > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > diff --git a/include/linux/bug.h b/include/linux/bug.h > > index aaac4bb..298a916 100644 > > --- a/include/linux/bug.h > > +++ b/include/linux/bug.h > > @@ -73,7 +73,7 @@ extern int __build_bug_on_failed; > > #define BUILD_BUG() \ > > do { \ > > extern void __build_bug_failed(void) \ > > - __linktime_error("BUILD_BUG failed"); \ > > + __compiletime_error("BUILD_BUG failed");\ > > __build_bug_failed(); \ > > } while (0) > > This change should either occur as part of patch 5 or before patch 5, > not after. I noticed the same thing and was about to comment on it. Please do not break bisectablity. All your patches should compile and run at every step. -- Steve -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Sep 28, 2012 at 09:04:35PM -0400, Steven Rostedt wrote: > On Fri, 2012-09-28 at 17:23 -0700, Josh Triplett wrote: > > On Fri, Sep 28, 2012 at 06:20:07PM -0500, Daniel Santos wrote: > > > Signed-off-by: Daniel Santos <daniel.santos@pobox.com> > > > --- > > > include/linux/bug.h | 2 +- > > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > > > diff --git a/include/linux/bug.h b/include/linux/bug.h > > > index aaac4bb..298a916 100644 > > > --- a/include/linux/bug.h > > > +++ b/include/linux/bug.h > > > @@ -73,7 +73,7 @@ extern int __build_bug_on_failed; > > > #define BUILD_BUG() \ > > > do { \ > > > extern void __build_bug_failed(void) \ > > > - __linktime_error("BUILD_BUG failed"); \ > > > + __compiletime_error("BUILD_BUG failed");\ > > > __build_bug_failed(); \ > > > } while (0) > > > > This change should either occur as part of patch 5 or before patch 5, > > not after. > > I noticed the same thing and was about to comment on it. > > Please do not break bisectablity. All your patches should compile and > run at every step. And while we're at it, every patch upstream should have a commit message explaining why this is done. No matter how trivial it is, because after a sufficient amount of time passes, everyone tends to forget why this has been done. Thanks.
On 09/30/2012 08:22 AM, Borislav Petkov wrote: > On Fri, Sep 28, 2012 at 09:04:35PM -0400, Steven Rostedt wrote: >> On Fri, 2012-09-28 at 17:23 -0700, Josh Triplett wrote: >>> On Fri, Sep 28, 2012 at 06:20:07PM -0500, Daniel Santos wrote: >>>> Signed-off-by: Daniel Santos <daniel.santos@pobox.com> >>>> --- >>>> include/linux/bug.h | 2 +- >>>> 1 files changed, 1 insertions(+), 1 deletions(-) >>>> >>>> diff --git a/include/linux/bug.h b/include/linux/bug.h >>>> index aaac4bb..298a916 100644 >>>> --- a/include/linux/bug.h >>>> +++ b/include/linux/bug.h >>>> @@ -73,7 +73,7 @@ extern int __build_bug_on_failed; >>>> #define BUILD_BUG() \ >>>> do { \ >>>> extern void __build_bug_failed(void) \ >>>> - __linktime_error("BUILD_BUG failed"); \ >>>> + __compiletime_error("BUILD_BUG failed");\ >>>> __build_bug_failed(); \ >>>> } while (0) >>> This change should either occur as part of patch 5 or before patch 5, >>> not after. >> I noticed the same thing and was about to comment on it. >> >> Please do not break bisectablity. All your patches should compile and >> run at every step. > And while we're at it, every patch upstream should have a commit message > explaining why this is done. No matter how trivial it is, because after > a sufficient amount of time passes, everyone tends to forget why this > has been done. > > Thanks. > Ah, well thank you all for the guidance! Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 28 Sep 2012, Daniel Santos wrote: > Signed-off-by: Daniel Santos <daniel.santos@pobox.com> After this is folded into the previous patch in the series, "compiler{,-gcc4}.h: Remove duplicate macros", then: Acked-by: David Rientjes <rientjes@google.com> -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/03/2012 01:44 AM, David Rientjes wrote: > On Fri, 28 Sep 2012, Daniel Santos wrote: > >> Signed-off-by: Daniel Santos <daniel.santos@pobox.com> > After this is folded into the previous patch in the series, > "compiler{,-gcc4}.h: Remove duplicate macros", then: > > Acked-by: David Rientjes <rientjes@google.com> Thanks. I've actually just reversed the patch order per Josh's suggestion and added patch comments to it. I can squash them if you guys prefer. Unfortunately, I'm a bit confused as to how I should re-submit these, still being new to this project. Patch 1 is already in -mm. Patches 2-3 have not changed. I've made a correction to patch #4 and reversed the order of 5 & 6. And what was 8-10 is now 8-15, as I've completely re-done BUILD_BUG_ON. I was planning on just submitting the whole set again, is this the correct protocol? If so, should I reply to the original [PATCH 0/10] thread or create a new one? Thanks! Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 03, 2012 at 06:49:10AM -0500, Daniel Santos wrote: > On 10/03/2012 01:44 AM, David Rientjes wrote: > > On Fri, 28 Sep 2012, Daniel Santos wrote: > > > >> Signed-off-by: Daniel Santos <daniel.santos@pobox.com> > > After this is folded into the previous patch in the series, > > "compiler{,-gcc4}.h: Remove duplicate macros", then: > > > > Acked-by: David Rientjes <rientjes@google.com> > Thanks. I've actually just reversed the patch order per Josh's > suggestion and added patch comments to it. I can squash them if you > guys prefer. > > Unfortunately, I'm a bit confused as to how I should re-submit these, > still being new to this project. Patch 1 is already in -mm. Patches 2-3 > have not changed. I've made a correction to patch #4 and reversed the > order of 5 & 6. And what was 8-10 is now 8-15, as I've completely > re-done BUILD_BUG_ON. I was planning on just submitting the whole set > again, is this the correct protocol? If so, should I reply to the > original [PATCH 0/10] thread or create a new one? Make your cover letter a reply to the original PATCH 0/10 mail, generate your patches with git format-patch --subject-prefix=PATCHv2 , and include in the cover letter a patch series changelog saying what changed in v2. - Josh Triplett -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 3 Oct 2012, Daniel Santos wrote: > Thanks. I've actually just reversed the patch order per Josh's > suggestion and added patch comments to it. I can squash them if you > guys prefer. > No need to be so fine-grained in your patches, if you're trying to replace __linktime_error with __compiletime_error, which happens to be the title of the patch (and should remain the title), then just remove it's single occurrence and its definition at the same time with a clear changelog that __compiletime_error is sufficient. No need to have two small patches with the same motivation. > Unfortunately, I'm a bit confused as to how I should re-submit these, > still being new to this project. Patch 1 is already in -mm. Patches 2-3 > have not changed. I've made a correction to patch #4 and reversed the > order of 5 & 6. And what was 8-10 is now 8-15, as I've completely > re-done BUILD_BUG_ON. I was planning on just submitting the whole set > again, is this the correct protocol? If so, should I reply to the > original [PATCH 0/10] thread or create a new one? > You already have a patch in -mm, so you have to base your series on that tree. Get the latest -mm tree from http://www.ozlabs.org/~akpm/mmotm/ and base the revised series on that tree, then send it off to Andrew Morton <akpm@linux-foundation.org> and cc the list and your reviewers. People often find it helpful to make it clear that this is v2 of the patchset and that it's based on -mm as a helpful pointer. -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/03/2012 01:26 PM, David Rientjes wrote: > On Wed, 3 Oct 2012, Daniel Santos wrote: > >> Thanks. I've actually just reversed the patch order per Josh's >> suggestion and added patch comments to it. I can squash them if you >> guys prefer. >> > No need to be so fine-grained in your patches, if you're trying to replace > __linktime_error with __compiletime_error, which happens to be the title > of the patch (and should remain the title), then just remove it's single > occurrence and its definition at the same time with a clear changelog that > __compiletime_error is sufficient. No need to have two small patches with > the same motivation. Sounds good to me > >> Unfortunately, I'm a bit confused as to how I should re-submit these, >> still being new to this project. Patch 1 is already in -mm. Patches 2-3 >> have not changed. I've made a correction to patch #4 and reversed the >> order of 5 & 6. And what was 8-10 is now 8-15, as I've completely >> re-done BUILD_BUG_ON. I was planning on just submitting the whole set >> again, is this the correct protocol? If so, should I reply to the >> original [PATCH 0/10] thread or create a new one? >> > You already have a patch in -mm, so you have to base your series on that > tree. Get the latest -mm tree from http://www.ozlabs.org/~akpm/mmotm/ and > base the revised series on that tree, then send it off to > Andrew Morton <akpm@linux-foundation.org> and cc the list and your > reviewers. People often find it helpful to make it clear that this is v2 > of the patchset and that it's based on -mm as a helpful pointer. I have it checked out from git://git.cmpxchg.org/linux-mmotm.git, the problem is that I cannot correctly test against that right now because I get an oops (without my patches) when setting up LVM (same on -next, bug report here https://bugzilla.kernel.org/show_bug.cgi?id=48241). What I'm thinking about doing is to rebase them against v3.6 again and test them there, but it will require a few minor changes (due to walken's patches not being present). Still, it's better than no re-testing. Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 3 Oct 2012, Daniel Santos wrote: > I have it checked out from git://git.cmpxchg.org/linux-mmotm.git, the > problem is that I cannot correctly test against that right now because I > get an oops (without my patches) when setting up LVM (same on -next, bug > report here https://bugzilla.kernel.org/show_bug.cgi?id=48241). What > I'm thinking about doing is to rebase them against v3.6 again and test > them there, but it will require a few minor changes (due to walken's > patches not being present). Still, it's better than no re-testing. > Daniel > I would cherry-pick the changes you already have for this patchset in -mm into Linus' latest tree, then base your new patchset on top of that modified tree. -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/bug.h b/include/linux/bug.h index aaac4bb..298a916 100644 --- a/include/linux/bug.h +++ b/include/linux/bug.h @@ -73,7 +73,7 @@ extern int __build_bug_on_failed; #define BUILD_BUG() \ do { \ extern void __build_bug_failed(void) \ - __linktime_error("BUILD_BUG failed"); \ + __compiletime_error("BUILD_BUG failed");\ __build_bug_failed(); \ } while (0)
Signed-off-by: Daniel Santos <daniel.santos@pobox.com> --- include/linux/bug.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)