[PATCH 0/2] xfs: fix inode count underrun

[PATCH 0/2] xfs: fix inode count underrun

[Dave Chinner]

Hi folks,

With the conversion of XFS to use the generic per-cpu superblocks, I
overlooked the fact that the update batch size is important to the
accuracy of the comparison function. Using different batch sizes
means percpu_counter_compare() doesn't detect when it should fall
back to percpu_counter_sum() for accuracy correctly, resulting in
counter comparisons being inaccurate. This leads to problems with
zero threshold detection in XFS.

To fix, add __percpu_counter_compare() to take a caller supplied
batch size. This fixes the XFS regression introduced in 4.1-rc1.

-Dave.

[PATCH 0/2] xfs: fix inode count underrun

[Dave Chinner]

Hi folks,

With the conversion of XFS to use the generic per-cpu superblocks, I
overlooked the fact that the update batch size is important to the
accuracy of the comparison function. Using different batch sizes
means percpu_counter_compare() doesn't detect when it should fall
back to percpu_counter_sum() for accuracy correctly, resulting in
counter comparisons being inaccurate. This leads to problems with
zero threshold detection in XFS.

To fix, add __percpu_counter_compare() to take a caller supplied
batch size. This fixes the XFS regression introduced in 4.1-rc1.

-Dave.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 1/2] percpu_counter: batch size aware __percpu_counter_compare()

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

From: Dave Chinner <dchinner@redhat.com>

XFS uses non-stanard batch sizes for avoiding frequent global
counter updates on it's allocated inode counters, as they increment
or decrement in batches of 64 inodes. Hence the standard percpu
counter batch of 32 means that the counter is effectively a global
counter. Currently Xfs uses a batch size of 128 so that it doesn't
take the global lock on every single modification.

However, Xfs also needs to compare accurately against zero, which
means we need to use percpu_counter_compare(), and that has a
hard-coded batch size of 32, and hence will spuriously fail to
detect when it is supposed to use precise comparisons and hence
the accounting goes wrong.

Add __percpu_counter_compare() to take a custom batch size so we can
use it sanely in XFS and factor percpu_counter_compare() to use it.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 include/linux/percpu_counter.h | 13 ++++++++++++-
 lib/percpu_counter.c           |  6 +++---
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/include/linux/percpu_counter.h b/include/linux/percpu_counter.h
index 50e5009..4c82e60 100644
--- a/include/linux/percpu_counter.h
+++ b/include/linux/percpu_counter.h
@@ -41,7 +41,12 @@ void percpu_counter_destroy(struct percpu_counter *fbc);
 void percpu_counter_set(struct percpu_counter *fbc, s64 amount);
 void __percpu_counter_add(struct percpu_counter *fbc, s64 amount, s32 batch);
 s64 __percpu_counter_sum(struct percpu_counter *fbc);
-int percpu_counter_compare(struct percpu_counter *fbc, s64 rhs);
+int __percpu_counter_compare(struct percpu_counter *fbc, s64 rhs, s32 batch);
+
+static inline int percpu_counter_compare(struct percpu_counter *fbc, s64 rhs)
+{
+	return __percpu_counter_compare(fbc, rhs, percpu_counter_batch);
+}
 
 static inline void percpu_counter_add(struct percpu_counter *fbc, s64 amount)
 {
@@ -116,6 +121,12 @@ static inline int percpu_counter_compare(struct percpu_counter *fbc, s64 rhs)
 		return 0;
 }
 
+static inline int
+percpu_counter_compare(struct percpu_counter *fbc, s64 rhs, s32 batch)
+{
+	return percpu_counter_compare(fbc, rhs);
+}
+
 static inline void
 percpu_counter_add(struct percpu_counter *fbc, s64 amount)
 {
diff --git a/lib/percpu_counter.c b/lib/percpu_counter.c
index 48144cd..f051d69 100644
--- a/lib/percpu_counter.c
+++ b/lib/percpu_counter.c
@@ -197,13 +197,13 @@ static int percpu_counter_hotcpu_callback(struct notifier_block *nb,
  * Compare counter against given value.
  * Return 1 if greater, 0 if equal and -1 if less
  */
-int percpu_counter_compare(struct percpu_counter *fbc, s64 rhs)
+int __percpu_counter_compare(struct percpu_counter *fbc, s64 rhs, s32 batch)
 {
 	s64	count;
 
 	count = percpu_counter_read(fbc);
 	/* Check to see if rough count will be sufficient for comparison */
-	if (abs(count - rhs) > (percpu_counter_batch*num_online_cpus())) {
+	if (abs(count - rhs) > (batch * num_online_cpus())) {
 		if (count > rhs)
 			return 1;
 		else
@@ -218,7 +218,7 @@ int percpu_counter_compare(struct percpu_counter *fbc, s64 rhs)
 	else
 		return 0;
 }
-EXPORT_SYMBOL(percpu_counter_compare);
+EXPORT_SYMBOL(__percpu_counter_compare);
 
 static int __init percpu_counter_startup(void)
 {
-- 
2.0.0

[PATCH 1/2] percpu_counter: batch size aware __percpu_counter_compare()

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

From: Dave Chinner <dchinner@redhat.com>

XFS uses non-stanard batch sizes for avoiding frequent global
counter updates on it's allocated inode counters, as they increment
or decrement in batches of 64 inodes. Hence the standard percpu
counter batch of 32 means that the counter is effectively a global
counter. Currently Xfs uses a batch size of 128 so that it doesn't
take the global lock on every single modification.

However, Xfs also needs to compare accurately against zero, which
means we need to use percpu_counter_compare(), and that has a
hard-coded batch size of 32, and hence will spuriously fail to
detect when it is supposed to use precise comparisons and hence
the accounting goes wrong.

Add __percpu_counter_compare() to take a custom batch size so we can
use it sanely in XFS and factor percpu_counter_compare() to use it.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 include/linux/percpu_counter.h | 13 ++++++++++++-
 lib/percpu_counter.c           |  6 +++---
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/include/linux/percpu_counter.h b/include/linux/percpu_counter.h
index 50e5009..4c82e60 100644
--- a/include/linux/percpu_counter.h
+++ b/include/linux/percpu_counter.h
@@ -41,7 +41,12 @@ void percpu_counter_destroy(struct percpu_counter *fbc);
 void percpu_counter_set(struct percpu_counter *fbc, s64 amount);
 void __percpu_counter_add(struct percpu_counter *fbc, s64 amount, s32 batch);
 s64 __percpu_counter_sum(struct percpu_counter *fbc);
-int percpu_counter_compare(struct percpu_counter *fbc, s64 rhs);
+int __percpu_counter_compare(struct percpu_counter *fbc, s64 rhs, s32 batch);
+
+static inline int percpu_counter_compare(struct percpu_counter *fbc, s64 rhs)
+{
+	return __percpu_counter_compare(fbc, rhs, percpu_counter_batch);
+}
 
 static inline void percpu_counter_add(struct percpu_counter *fbc, s64 amount)
 {
@@ -116,6 +121,12 @@ static inline int percpu_counter_compare(struct percpu_counter *fbc, s64 rhs)
 		return 0;
 }
 
+static inline int
+percpu_counter_compare(struct percpu_counter *fbc, s64 rhs, s32 batch)
+{
+	return percpu_counter_compare(fbc, rhs);
+}
+
 static inline void
 percpu_counter_add(struct percpu_counter *fbc, s64 amount)
 {
diff --git a/lib/percpu_counter.c b/lib/percpu_counter.c
index 48144cd..f051d69 100644
--- a/lib/percpu_counter.c
+++ b/lib/percpu_counter.c
@@ -197,13 +197,13 @@ static int percpu_counter_hotcpu_callback(struct notifier_block *nb,
  * Compare counter against given value.
  * Return 1 if greater, 0 if equal and -1 if less
  */
-int percpu_counter_compare(struct percpu_counter *fbc, s64 rhs)
+int __percpu_counter_compare(struct percpu_counter *fbc, s64 rhs, s32 batch)
 {
 	s64	count;
 
 	count = percpu_counter_read(fbc);
 	/* Check to see if rough count will be sufficient for comparison */
-	if (abs(count - rhs) > (percpu_counter_batch*num_online_cpus())) {
+	if (abs(count - rhs) > (batch * num_online_cpus())) {
 		if (count > rhs)
 			return 1;
 		else
@@ -218,7 +218,7 @@ int percpu_counter_compare(struct percpu_counter *fbc, s64 rhs)
 	else
 		return 0;
 }
-EXPORT_SYMBOL(percpu_counter_compare);
+EXPORT_SYMBOL(__percpu_counter_compare);
 
 static int __init percpu_counter_startup(void)
 {
-- 
2.0.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/2] xfs: inode counter needs to use __percpu_counter_compare

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

From: Dave Chinner <dchinner@redhat.com>

Because the counter uses a custom batch size, the comparison
function needs to be aware of that batch size otherwise the
comparison does not work correctly. This leads to ASSERT failures
on generic/027 like this:

 XFS: Assertion failed: 0, file: fs/xfs/xfs_mount.c, line: 1099
 ------------[ cut here ]------------
....
 Call Trace:
  [<ffffffff81522a39>] xfs_mod_icount+0x99/0xc0
  [<ffffffff815285cb>] xfs_trans_unreserve_and_mod_sb+0x28b/0x5b0
  [<ffffffff8152f941>] xfs_log_commit_cil+0x321/0x580
  [<ffffffff81528e17>] xfs_trans_commit+0xb7/0x260
  [<ffffffff81503d4d>] xfs_bmap_finish+0xcd/0x1b0
  [<ffffffff8151da41>] xfs_inactive_ifree+0x1e1/0x250
  [<ffffffff8151dbe0>] xfs_inactive+0x130/0x200
  [<ffffffff81523a21>] xfs_fs_evict_inode+0x91/0xf0
  [<ffffffff811f3958>] evict+0xb8/0x190
  [<ffffffff811f433b>] iput+0x18b/0x1f0
  [<ffffffff811e8853>] do_unlinkat+0x1f3/0x320
  [<ffffffff811d548a>] ? filp_close+0x5a/0x80
  [<ffffffff811e999b>] SyS_unlinkat+0x1b/0x40
  [<ffffffff81e0892e>] system_call_fastpath+0x12/0x71

This is a regression introduced by commit 501ab32 ("xfs: use generic
percpu counters for inode counter").

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_mount.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 02f827f..900f8ce 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -1106,8 +1106,9 @@ xfs_mod_icount(
 	int64_t			delta)
 {
 	/* deltas are +/-64, hence the large batch size of 128. */
-	__percpu_counter_add(&mp->m_icount, delta, 128);
-	if (percpu_counter_compare(&mp->m_icount, 0) < 0) {
+#define _ICOUNT_BATCH	128
+	__percpu_counter_add(&mp->m_icount, delta, _ICOUNT_BATCH);
+	if (__percpu_counter_compare(&mp->m_icount, 0, _ICOUNT_BATCH) < 0) {
 		ASSERT(0);
 		percpu_counter_add(&mp->m_icount, -delta);
 		return -EINVAL;
-- 
2.0.0

[PATCH 2/2] xfs: inode counter needs to use __percpu_counter_compare

[Dave Chinner]

From: Dave Chinner <dchinner@redhat.com>

From: Dave Chinner <dchinner@redhat.com>

Because the counter uses a custom batch size, the comparison
function needs to be aware of that batch size otherwise the
comparison does not work correctly. This leads to ASSERT failures
on generic/027 like this:

 XFS: Assertion failed: 0, file: fs/xfs/xfs_mount.c, line: 1099
 ------------[ cut here ]------------
....
 Call Trace:
  [<ffffffff81522a39>] xfs_mod_icount+0x99/0xc0
  [<ffffffff815285cb>] xfs_trans_unreserve_and_mod_sb+0x28b/0x5b0
  [<ffffffff8152f941>] xfs_log_commit_cil+0x321/0x580
  [<ffffffff81528e17>] xfs_trans_commit+0xb7/0x260
  [<ffffffff81503d4d>] xfs_bmap_finish+0xcd/0x1b0
  [<ffffffff8151da41>] xfs_inactive_ifree+0x1e1/0x250
  [<ffffffff8151dbe0>] xfs_inactive+0x130/0x200
  [<ffffffff81523a21>] xfs_fs_evict_inode+0x91/0xf0
  [<ffffffff811f3958>] evict+0xb8/0x190
  [<ffffffff811f433b>] iput+0x18b/0x1f0
  [<ffffffff811e8853>] do_unlinkat+0x1f3/0x320
  [<ffffffff811d548a>] ? filp_close+0x5a/0x80
  [<ffffffff811e999b>] SyS_unlinkat+0x1b/0x40
  [<ffffffff81e0892e>] system_call_fastpath+0x12/0x71

This is a regression introduced by commit 501ab32 ("xfs: use generic
percpu counters for inode counter").

Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 fs/xfs/xfs_mount.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 02f827f..900f8ce 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -1106,8 +1106,9 @@ xfs_mod_icount(
 	int64_t			delta)
 {
 	/* deltas are +/-64, hence the large batch size of 128. */
-	__percpu_counter_add(&mp->m_icount, delta, 128);
-	if (percpu_counter_compare(&mp->m_icount, 0) < 0) {
+#define _ICOUNT_BATCH	128
+	__percpu_counter_add(&mp->m_icount, delta, _ICOUNT_BATCH);
+	if (__percpu_counter_compare(&mp->m_icount, 0, _ICOUNT_BATCH) < 0) {
 		ASSERT(0);
 		percpu_counter_add(&mp->m_icount, -delta);
 		return -EINVAL;
-- 
2.0.0

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/2] percpu_counter: batch size aware __percpu_counter_compare()

[Christoph Hellwig]

This looks reasonable, but I miss Ccs to the authors of the
percpu_counter code and linux-kernel.

On Wed, May 06, 2015 at 08:01:38AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> From: Dave Chinner <dchinner@redhat.com>

And this looks wrong, too :)

Re: [PATCH 1/2] percpu_counter: batch size aware __percpu_counter_compare()

[Christoph Hellwig]

This looks reasonable, but I miss Ccs to the authors of the
percpu_counter code and linux-kernel.

On Wed, May 06, 2015 at 08:01:38AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> From: Dave Chinner <dchinner@redhat.com>

And this looks wrong, too :)

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/2] xfs: inode counter needs to use __percpu_counter_compare

[Christoph Hellwig]

> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index 02f827f..900f8ce 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -1106,8 +1106,9 @@ xfs_mod_icount(
>  	int64_t			delta)
>  {
>  	/* deltas are +/-64, hence the large batch size of 128. */
> -	__percpu_counter_add(&mp->m_icount, delta, 128);
> -	if (percpu_counter_compare(&mp->m_icount, 0) < 0) {
> +#define _ICOUNT_BATCH	128
> +	__percpu_counter_add(&mp->m_icount, delta, _ICOUNT_BATCH);

Can you give XFS_ prefixes to the atch sizes and move them otuside the
function?  And fix up the instance in xfs_mod_fdblocks as well while
you're at it.

Otherwise looks fine.

Re: [PATCH 2/2] xfs: inode counter needs to use __percpu_counter_compare

[Christoph Hellwig]

> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index 02f827f..900f8ce 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -1106,8 +1106,9 @@ xfs_mod_icount(
>  	int64_t			delta)
>  {
>  	/* deltas are +/-64, hence the large batch size of 128. */
> -	__percpu_counter_add(&mp->m_icount, delta, 128);
> -	if (percpu_counter_compare(&mp->m_icount, 0) < 0) {
> +#define _ICOUNT_BATCH	128
> +	__percpu_counter_add(&mp->m_icount, delta, _ICOUNT_BATCH);

Can you give XFS_ prefixes to the atch sizes and move them otuside the
function?  And fix up the instance in xfs_mod_fdblocks as well while
you're at it.

Otherwise looks fine.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/2] percpu_counter: batch size aware __percpu_counter_compare()

[Christoph Hellwig]

On Tue, May 05, 2015 at 09:36:36PM -0700, Christoph Hellwig wrote:
> This looks reasonable, but I miss Ccs to the authors of the
> percpu_counter code and linux-kernel.

Ok, saw it on lkml.  Guess I need more coffee, sorry..

Re: [PATCH 1/2] percpu_counter: batch size aware __percpu_counter_compare()

[Christoph Hellwig]

On Tue, May 05, 2015 at 09:36:36PM -0700, Christoph Hellwig wrote:
> This looks reasonable, but I miss Ccs to the authors of the
> percpu_counter code and linux-kernel.

Ok, saw it on lkml.  Guess I need more coffee, sorry..

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/2] percpu_counter: batch size aware __percpu_counter_compare()

[Dave Chinner]

On Tue, May 05, 2015 at 09:36:36PM -0700, Christoph Hellwig wrote:
> This looks reasonable, but I miss Ccs to the authors of the
> percpu_counter code and linux-kernel.

linux-kernel was cc'd.

> On Wed, May 06, 2015 at 08:01:38AM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > From: Dave Chinner <dchinner@redhat.com>
> 
> And this looks wrong, too :)

That looks like a change of tool behaviour. I just upgraded guilt;
the old version stripped "from" lines in patch descriptions from the
git commit log as goes into the author field of the commit. The new
version does not appear to strip the from lines, and so when
git-send-email formats it, it puts a from line in the message, and
then the commit message has one too.

Thanks for pointing it out.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 1/2] percpu_counter: batch size aware __percpu_counter_compare()

[Dave Chinner]

On Tue, May 05, 2015 at 09:36:36PM -0700, Christoph Hellwig wrote:
> This looks reasonable, but I miss Ccs to the authors of the
> percpu_counter code and linux-kernel.

linux-kernel was cc'd.

> On Wed, May 06, 2015 at 08:01:38AM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > From: Dave Chinner <dchinner@redhat.com>
> 
> And this looks wrong, too :)

That looks like a change of tool behaviour. I just upgraded guilt;
the old version stripped "from" lines in patch descriptions from the
git commit log as goes into the author field of the commit. The new
version does not appear to strip the from lines, and so when
git-send-email formats it, it puts a from line in the message, and
then the commit message has one too.

Thanks for pointing it out.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 2/2] xfs: inode counter needs to use __percpu_counter_compare

[Dave Chinner]

On Tue, May 05, 2015 at 09:38:07PM -0700, Christoph Hellwig wrote:
> > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> > index 02f827f..900f8ce 100644
> > --- a/fs/xfs/xfs_mount.c
> > +++ b/fs/xfs/xfs_mount.c
> > @@ -1106,8 +1106,9 @@ xfs_mod_icount(
> >  	int64_t			delta)
> >  {
> >  	/* deltas are +/-64, hence the large batch size of 128. */
> > -	__percpu_counter_add(&mp->m_icount, delta, 128);
> > -	if (percpu_counter_compare(&mp->m_icount, 0) < 0) {
> > +#define _ICOUNT_BATCH	128
> > +	__percpu_counter_add(&mp->m_icount, delta, _ICOUNT_BATCH);
> 
> Can you give XFS_ prefixes to the atch sizes and move them otuside the
> function?  And fix up the instance in xfs_mod_fdblocks as well while
> you're at it.

Sure.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 2/2] xfs: inode counter needs to use __percpu_counter_compare

[Dave Chinner]

On Tue, May 05, 2015 at 09:38:07PM -0700, Christoph Hellwig wrote:
> > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> > index 02f827f..900f8ce 100644
> > --- a/fs/xfs/xfs_mount.c
> > +++ b/fs/xfs/xfs_mount.c
> > @@ -1106,8 +1106,9 @@ xfs_mod_icount(
> >  	int64_t			delta)
> >  {
> >  	/* deltas are +/-64, hence the large batch size of 128. */
> > -	__percpu_counter_add(&mp->m_icount, delta, 128);
> > -	if (percpu_counter_compare(&mp->m_icount, 0) < 0) {
> > +#define _ICOUNT_BATCH	128
> > +	__percpu_counter_add(&mp->m_icount, delta, _ICOUNT_BATCH);
> 
> Can you give XFS_ prefixes to the atch sizes and move them otuside the
> function?  And fix up the instance in xfs_mod_fdblocks as well while
> you're at it.

Sure.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/2] percpu_counter: batch size aware __percpu_counter_compare()

[Dave Chinner]

On Wed, May 06, 2015 at 03:43:10PM +1000, Dave Chinner wrote:
> On Tue, May 05, 2015 at 09:36:36PM -0700, Christoph Hellwig wrote:
> > This looks reasonable, but I miss Ccs to the authors of the
> > percpu_counter code and linux-kernel.
> 
> linux-kernel was cc'd.
> 
> > On Wed, May 06, 2015 at 08:01:38AM +1000, Dave Chinner wrote:
> > > From: Dave Chinner <dchinner@redhat.com>
> > > 
> > > From: Dave Chinner <dchinner@redhat.com>
> > 
> > And this looks wrong, too :)
> 
> That looks like a change of tool behaviour. I just upgraded guilt;
> the old version stripped "from" lines in patch descriptions from the
> git commit log as goes into the author field of the commit. The new
> version does not appear to strip the from lines, and so when
> git-send-email formats it, it puts a from line in the message, and
> then the commit message has one too.

Ok, so I remembered it the wrong way around, but it is a tool bug:

$ guilt patchbomb -s d480578..cd83c77
cd83c773 xfs: inode counter needs to use __percpu_counter_compare
1e41b5ad percpu_counter: batch size aware __percpu_counter_compare()
Are these what you want to send? [Y/n]
....

Note the use of "-s". from the man page:

       -s
	   Don't add additional repository committer sign-offs to
	   the patch.  This allows the sign-off chain to be fully
	   expressed in the commit messages and not changed by the
	   act of sending a patchbomb.

It would appear that the "-s" option is not working properly. I'll
fix that before sending the next round of patches....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 1/2] percpu_counter: batch size aware __percpu_counter_compare()

[Dave Chinner]

On Wed, May 06, 2015 at 03:43:10PM +1000, Dave Chinner wrote:
> On Tue, May 05, 2015 at 09:36:36PM -0700, Christoph Hellwig wrote:
> > This looks reasonable, but I miss Ccs to the authors of the
> > percpu_counter code and linux-kernel.
> 
> linux-kernel was cc'd.
> 
> > On Wed, May 06, 2015 at 08:01:38AM +1000, Dave Chinner wrote:
> > > From: Dave Chinner <dchinner@redhat.com>
> > > 
> > > From: Dave Chinner <dchinner@redhat.com>
> > 
> > And this looks wrong, too :)
> 
> That looks like a change of tool behaviour. I just upgraded guilt;
> the old version stripped "from" lines in patch descriptions from the
> git commit log as goes into the author field of the commit. The new
> version does not appear to strip the from lines, and so when
> git-send-email formats it, it puts a from line in the message, and
> then the commit message has one too.

Ok, so I remembered it the wrong way around, but it is a tool bug:

$ guilt patchbomb -s d480578..cd83c77
cd83c773 xfs: inode counter needs to use __percpu_counter_compare
1e41b5ad percpu_counter: batch size aware __percpu_counter_compare()
Are these what you want to send? [Y/n]
....

Note the use of "-s". from the man page:

       -s
	   Don't add additional repository committer sign-offs to
	   the patch.  This allows the sign-off chain to be fully
	   expressed in the commit messages and not changed by the
	   act of sending a patchbomb.

It would appear that the "-s" option is not working properly. I'll
fix that before sending the next round of patches....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/2] percpu_counter: batch size aware __percpu_counter_compare()

[Dave Chinner]

On Wed, May 06, 2015 at 03:53:49PM +1000, Dave Chinner wrote:
> On Wed, May 06, 2015 at 03:43:10PM +1000, Dave Chinner wrote:
> > On Tue, May 05, 2015 at 09:36:36PM -0700, Christoph Hellwig wrote:
> > > This looks reasonable, but I miss Ccs to the authors of the
> > > percpu_counter code and linux-kernel.
> > 
> > linux-kernel was cc'd.
> > 
> > > On Wed, May 06, 2015 at 08:01:38AM +1000, Dave Chinner wrote:
> > > > From: Dave Chinner <dchinner@redhat.com>
> > > > 
> > > > From: Dave Chinner <dchinner@redhat.com>
> > > 
> > > And this looks wrong, too :)
> > 
> > That looks like a change of tool behaviour. I just upgraded guilt;
> > the old version stripped "from" lines in patch descriptions from the
> > git commit log as goes into the author field of the commit. The new
> > version does not appear to strip the from lines, and so when
> > git-send-email formats it, it puts a from line in the message, and
> > then the commit message has one too.
> 
> Ok, so I remembered it the wrong way around, but it is a tool bug:
> 
> $ guilt patchbomb -s d480578..cd83c77
> cd83c773 xfs: inode counter needs to use __percpu_counter_compare
> 1e41b5ad percpu_counter: batch size aware __percpu_counter_compare()
> Are these what you want to send? [Y/n]
> ....
> 
> Note the use of "-s". from the man page:
> 
>        -s
> 	   Don't add additional repository committer sign-offs to
> 	   the patch.  This allows the sign-off chain to be fully
> 	   expressed in the commit messages and not changed by the
> 	   act of sending a patchbomb.
> 
> It would appear that the "-s" option is not working properly. I'll
> fix that before sending the next round of patches....

<sigh>

So, the packaged version of guilt on a brand new debian unstable
installation is actually older than the 2 year version of guilt I
had previously installed and used on a different machine. IOWs, the
debian distro version has this bug in it:

http://repo.or.cz/w/guilt.git?a=commit;h=8f88f953580a0cacf111bf64c0e838014ee30c01

Yeah, that was fixed more than 3 years ago. Oh well, back to using
a locally installed version...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 1/2] percpu_counter: batch size aware __percpu_counter_compare()

[Dave Chinner]

On Wed, May 06, 2015 at 03:53:49PM +1000, Dave Chinner wrote:
> On Wed, May 06, 2015 at 03:43:10PM +1000, Dave Chinner wrote:
> > On Tue, May 05, 2015 at 09:36:36PM -0700, Christoph Hellwig wrote:
> > > This looks reasonable, but I miss Ccs to the authors of the
> > > percpu_counter code and linux-kernel.
> > 
> > linux-kernel was cc'd.
> > 
> > > On Wed, May 06, 2015 at 08:01:38AM +1000, Dave Chinner wrote:
> > > > From: Dave Chinner <dchinner@redhat.com>
> > > > 
> > > > From: Dave Chinner <dchinner@redhat.com>
> > > 
> > > And this looks wrong, too :)
> > 
> > That looks like a change of tool behaviour. I just upgraded guilt;
> > the old version stripped "from" lines in patch descriptions from the
> > git commit log as goes into the author field of the commit. The new
> > version does not appear to strip the from lines, and so when
> > git-send-email formats it, it puts a from line in the message, and
> > then the commit message has one too.
> 
> Ok, so I remembered it the wrong way around, but it is a tool bug:
> 
> $ guilt patchbomb -s d480578..cd83c77
> cd83c773 xfs: inode counter needs to use __percpu_counter_compare
> 1e41b5ad percpu_counter: batch size aware __percpu_counter_compare()
> Are these what you want to send? [Y/n]
> ....
> 
> Note the use of "-s". from the man page:
> 
>        -s
> 	   Don't add additional repository committer sign-offs to
> 	   the patch.  This allows the sign-off chain to be fully
> 	   expressed in the commit messages and not changed by the
> 	   act of sending a patchbomb.
> 
> It would appear that the "-s" option is not working properly. I'll
> fix that before sending the next round of patches....

<sigh>

So, the packaged version of guilt on a brand new debian unstable
installation is actually older than the 2 year version of guilt I
had previously installed and used on a different machine. IOWs, the
debian distro version has this bug in it:

http://repo.or.cz/w/guilt.git?a=commit;h=8f88f953580a0cacf111bf64c0e838014ee30c01

Yeah, that was fixed more than 3 years ago. Oh well, back to using
a locally installed version...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs