[PATCH] dm-statistics: report precise timestamps and histogram

[PATCH] dm-statistics: report precise timestamps and histogram

[Mikulas Patocka]

Mike, please submit this patch to the upstream kernel before 4.2 is 
released, so that the all the kernels that support the options 
"precise_timestamps" and "histogram" also report these values back.


From: Mikulas Patocka <mpatocka@redhat.com>

If the user selected precise timestamps or histogram, report it in the
output of the @stats_list message.

If the user didn't select these options, no extra tokens are reported,
thus it is backward compatible with old software that doesn't know about
precise timestamps and histogram.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

Index: linux-4.1.5/Documentation/device-mapper/statistics.txt
===================================================================
--- linux-4.1.5.orig/Documentation/device-mapper/statistics.txt
+++ linux-4.1.5/Documentation/device-mapper/statistics.txt
@@ -118,6 +118,10 @@ Messages
 
 	Output format:
 	  <region_id>: <start_sector>+<length> <step> <program_id> <aux_data>
+	  	precise_timestamps histogram:n1,n2,n3,...
+
+	The strings "precise_timestamps" and "histogram" are printed only
+	if they were specified when creating the region.
 
     @stats_print <region_id> [<starting_line> <number_of_lines>]
 
Index: linux-4.1.5/drivers/md/dm-stats.c
===================================================================
--- linux-4.1.5.orig/drivers/md/dm-stats.c
+++ linux-4.1.5/drivers/md/dm-stats.c
@@ -453,12 +453,24 @@ static int dm_stats_list(struct dm_stats
 	list_for_each_entry(s, &stats->list, list_entry) {
 		if (!program || !strcmp(program, s->program_id)) {
 			len = s->end - s->start;
-			DMEMIT("%d: %llu+%llu %llu %s %s\n", s->id,
+			DMEMIT("%d: %llu+%llu %llu %s %s", s->id,
 				(unsigned long long)s->start,
 				(unsigned long long)len,
 				(unsigned long long)s->step,
 				s->program_id,
 				s->aux_data);
+			if (s->stat_flags & STAT_PRECISE_TIMESTAMPS)
+				DMEMIT(" precise_timestamps");
+			if (s->n_histogram_entries) {
+				unsigned i;
+				DMEMIT(" histogram:");
+				for (i = 0; i < s->n_histogram_entries; i++) {
+					if (i)
+						DMEMIT(",");
+					DMEMIT("%llu", s->histogram_boundaries[i]);
+				}
+			}
+			DMEMIT("\n");
 		}
 	}
 	mutex_unlock(&stats->mutex);

Re: dm-statistics: report precise timestamps and histogram

[Mike Snitzer]

On Tue, Aug 18 2015 at  4:26pm -0400,
Mikulas Patocka <mpatocka@redhat.com> wrote:

> Mike, please submit this patch to the upstream kernel before 4.2 is 
> released, so that the all the kernels that support the options 
> "precise_timestamps" and "histogram" also report these values back.

It is really late to be sending pull requests for 4.2 (next release is
likely 4.2 final).  I've already sent 6 'fixes' pull requests to Linus
for 4.2.  This patch is just too late, sorry.

But what I will do is tag this commit for 4.2-stable inclusion.

Userspace will have to check the DM ioctl version to know if DM provides
these extra keywords.

Mike

Re: dm-statistics: report precise timestamps and histogram

[Mike Snitzer]

On Tue, Aug 18 2015 at  5:07pm -0400,
Mike Snitzer <snitzer@redhat.com> wrote:

> On Tue, Aug 18 2015 at  4:26pm -0400,
> Mikulas Patocka <mpatocka@redhat.com> wrote:
> 
> > Mike, please submit this patch to the upstream kernel before 4.2 is 
> > released, so that the all the kernels that support the options 
> > "precise_timestamps" and "histogram" also report these values back.
> 
> It is really late to be sending pull requests for 4.2 (next release is
> likely 4.2 final).  I've already sent 6 'fixes' pull requests to Linus
> for 4.2.  This patch is just too late, sorry.
> 
> But what I will do is tag this commit for 4.2-stable inclusion.
> 
> Userspace will have to check the DM ioctl version to know if DM provides
> these extra keywords.

I've staged the following for 4.3:
https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=bd49784fd1e8f42c7600fbfa206361324857f373

Bryn, please verify this change is sufficient for your needs.

Re: [PATCH] dm-statistics: report precise timestamps and histogram

[Bryn M. Reeves]

On Tue, Aug 18, 2015 at 04:26:16PM -0400, Mikulas Patocka wrote:
> If the user selected precise timestamps or histogram, report it in the
> output of the @stats_list message.
> 
> If the user didn't select these options, no extra tokens are reported,
> thus it is backward compatible with old software that doesn't know about
> precise timestamps and histogram.

It's not strictly backwards compatible in that someone mixing old and
new code could have a situation where "old" dm-stats parsing code could
see entries in list output created using "new" features.

This would only affect users not implementing program_id filtering (or
using it in broken ways) though so it's probably insignificant enough to
be ignored.

Regards,
Bryn.

Re: dm-statistics: report precise timestamps and histogram

[Bryn M. Reeves]

On Tue, Aug 18, 2015 at 05:41:12PM -0400, Mike Snitzer wrote:
> On Tue, Aug 18 2015 at  5:07pm -0400,
> Mike Snitzer <snitzer@redhat.com> wrote:
> 
> > On Tue, Aug 18 2015 at  4:26pm -0400,
> > Mikulas Patocka <mpatocka@redhat.com> wrote:
> > 
> > > Mike, please submit this patch to the upstream kernel before 4.2 is 
> > > released, so that the all the kernels that support the options 
> > > "precise_timestamps" and "histogram" also report these values back.
> > 
> > It is really late to be sending pull requests for 4.2 (next release is
> > likely 4.2 final).  I've already sent 6 'fixes' pull requests to Linus
> > for 4.2.  This patch is just too late, sorry.
> > 
> > But what I will do is tag this commit for 4.2-stable inclusion.
> > 
> > Userspace will have to check the DM ioctl version to know if DM provides
> > these extra keywords.
> 
> I've staged the following for 4.3:
> https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=bd49784fd1e8f42c7600fbfa206361324857f373
> 
> Bryn, please verify this change is sufficient for your needs.

Yes; this will be usable by dmstats.

Unfortuntely the fact that there are kernels out there without this
means that we will need to either carry a workaround in dmstats
userspace forever or just disallow use of these features on those
kernels.

The workaround is fairly unobtrusive and is already written so it's not
the end of the world (it also has future uses for which kernel support
will not help so it won't be solely used for precise/histogram
tracking).

Regards,
Bryn.

Re: dm-statistics: report precise timestamps and histogram

[Mikulas Patocka]

On Wed, 19 Aug 2015, Bryn M. Reeves wrote:

> On Tue, Aug 18, 2015 at 05:41:12PM -0400, Mike Snitzer wrote:
> > On Tue, Aug 18 2015 at  5:07pm -0400,
> > Mike Snitzer <snitzer@redhat.com> wrote:
> > 
> > > On Tue, Aug 18 2015 at  4:26pm -0400,
> > > Mikulas Patocka <mpatocka@redhat.com> wrote:
> > > 
> > > > Mike, please submit this patch to the upstream kernel before 4.2 is 
> > > > released, so that the all the kernels that support the options 
> > > > "precise_timestamps" and "histogram" also report these values back.
> > > 
> > > It is really late to be sending pull requests for 4.2 (next release is
> > > likely 4.2 final).  I've already sent 6 'fixes' pull requests to Linus
> > > for 4.2.  This patch is just too late, sorry.
> > > 
> > > But what I will do is tag this commit for 4.2-stable inclusion.
> > > 
> > > Userspace will have to check the DM ioctl version to know if DM provides
> > > these extra keywords.
> > 
> > I've staged the following for 4.3:
> > https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=bd49784fd1e8f42c7600fbfa206361324857f373
> > 
> > Bryn, please verify this change is sufficient for your needs.
> 
> Yes; this will be usable by dmstats.
> 
> Unfortuntely the fact that there are kernels out there without this
> means that we will need to either carry a workaround in dmstats
> userspace forever or just disallow use of these features on those
> kernels.

That's why I think it should be submitted to 4.2 now - so that there won't 
be an upstream kernel without this feature.

> The workaround is fairly unobtrusive and is already written so it's not
> the end of the world (it also has future uses for which kernel support
> will not help so it won't be solely used for precise/histogram
> tracking).
> 
> Regards,
> Bryn.

Mikulas

Re: dm-statistics: report precise timestamps and histogram

[Bryn M. Reeves]

On Wed, Aug 19, 2015 at 06:50:36AM -0400, Mikulas Patocka wrote:
> On Wed, 19 Aug 2015, Bryn M. Reeves wrote:
> > On Tue, Aug 18, 2015 at 05:41:12PM -0400, Mike Snitzer wrote:
> > > On Tue, Aug 18 2015 at  5:07pm -0400,
> > > Mike Snitzer <snitzer@redhat.com> wrote:
> > > > But what I will do is tag this commit for 4.2-stable inclusion.
> > > > 
> > > > Userspace will have to check the DM ioctl version to know if DM provides
> > > > these extra keywords.
> > > 
> > > I've staged the following for 4.3:
> > > https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=bd49784fd1e8f42c7600fbfa206361324857f373
> > > 
> > Unfortuntely the fact that there are kernels out there without this
> > means that we will need to either carry a workaround in dmstats
> > userspace forever or just disallow use of these features on those
> > kernels.
> 
> That's why I think it should be submitted to 4.2 now - so that there won't 
> be an upstream kernel without this feature.

Agreed: that's the ideal situation.

Regards,
Bryn.

Re: dm-statistics: report precise timestamps and histogram

[Mikulas Patocka]

On Tue, 18 Aug 2015, Mike Snitzer wrote:

> On Tue, Aug 18 2015 at  4:26pm -0400,
> Mikulas Patocka <mpatocka@redhat.com> wrote:
> 
> > Mike, please submit this patch to the upstream kernel before 4.2 is 
> > released, so that the all the kernels that support the options 
> > "precise_timestamps" and "histogram" also report these values back.
> 
> It is really late to be sending pull requests for 4.2 (next release is
> likely 4.2 final).  I've already sent 6 'fixes' pull requests to Linus
> for 4.2.  This patch is just too late, sorry.
> 
> But what I will do is tag this commit for 4.2-stable inclusion.
> 
> Userspace will have to check the DM ioctl version to know if DM provides
> these extra keywords.
> 
> Mike

But if you don't send it, userspace will forever have to deal with kernels 
that don't report this feature back.

You can think that not reporting these features was a bug in the original 
design and this patch fixes the bug.

Mikulas

Re: dm-statistics: report precise timestamps and histogram

[Bryn M. Reeves]

On Wed, Aug 19, 2015 at 06:53:41AM -0400, Mikulas Patocka wrote:
> On Tue, 18 Aug 2015, Mike Snitzer wrote:
> > On Tue, Aug 18 2015 at  4:26pm -0400,
> > Mikulas Patocka <mpatocka@redhat.com> wrote:
> > > Mike, please submit this patch to the upstream kernel before 4.2 is 
> > > released, so that the all the kernels that support the options 
> > > "precise_timestamps" and "histogram" also report these values back.
> > 
> > Userspace will have to check the DM ioctl version to know if DM provides
> > these extra keywords.
> > 
> > Mike
> 
> But if you don't send it, userspace will forever have to deal with kernels 
> that don't report this feature back.

We'd either end up saying some kernels can't support it when they can, or
have to carry some additional userspace clutter forever to provide a 
workaround.

Regards,
Bryn.
 

Re: dm-statistics: report precise timestamps and histogram

[Mike Snitzer]

On Wed, Aug 19 2015 at  7:11am -0400,
Bryn M. Reeves <bmr@redhat.com> wrote:

> On Wed, Aug 19, 2015 at 06:53:41AM -0400, Mikulas Patocka wrote:
> > On Tue, 18 Aug 2015, Mike Snitzer wrote:
> > > On Tue, Aug 18 2015 at  4:26pm -0400,
> > > Mikulas Patocka <mpatocka@redhat.com> wrote:
> > > > Mike, please submit this patch to the upstream kernel before 4.2 is 
> > > > released, so that the all the kernels that support the options 
> > > > "precise_timestamps" and "histogram" also report these values back.
> > > 
> > > Userspace will have to check the DM ioctl version to know if DM provides
> > > these extra keywords.
> > > 
> > > Mike
> > 
> > But if you don't send it, userspace will forever have to deal with kernels 
> > that don't report this feature back.
> 
> We'd either end up saying some kernels can't support it when they can, or
> have to carry some additional userspace clutter forever to provide a 
> workaround.

As much as I'd like to push this in the final hour of 4.2's release I'm
sorry that I cannot.  It is too late given it is _not_ a critical fix.

I've bumped the version to .33 and marked this change for stable, just
have lvm2 check for that version in order to support these features.
Yes, it translates to the Linux 4.2.0 release not being supported but
any stable 4.2-stable release will be.

Mike

Re: dm-statistics: report precise timestamps and histogram

[Mikulas Patocka]

On Wed, 19 Aug 2015, Bryn M. Reeves wrote:

> On Wed, Aug 19, 2015 at 06:53:41AM -0400, Mikulas Patocka wrote:
> > On Tue, 18 Aug 2015, Mike Snitzer wrote:
> > > On Tue, Aug 18 2015 at  4:26pm -0400,
> > > Mikulas Patocka <mpatocka@redhat.com> wrote:
> > > > Mike, please submit this patch to the upstream kernel before 4.2 is 
> > > > released, so that the all the kernels that support the options 
> > > > "precise_timestamps" and "histogram" also report these values back.
> > > 
> > > Userspace will have to check the DM ioctl version to know if DM provides
> > > these extra keywords.
> > > 
> > > Mike
> > 
> > But if you don't send it, userspace will forever have to deal with kernels 
> > that don't report this feature back.
> 
> We'd either end up saying some kernels can't support it when they can, or
> have to carry some additional userspace clutter forever to provide a 
> workaround.
> 
> Regards,
> Bryn.

I think you don't need to provide a workaround for kernels that don't 
report it. The workaround would be needed just for 4.2.0 (Mike said he 
will backport the fix to 4.2.1), there is no need to bloat userspace with 
that.

Mikulas

Re: dm-statistics: report precise timestamps and histogram

[Bryn M. Reeves]

On Thu, Aug 20, 2015 at 03:40:10PM -0400, Mikulas Patocka wrote:
> 
> 
> On Wed, 19 Aug 2015, Bryn M. Reeves wrote:
> 
> > On Wed, Aug 19, 2015 at 06:53:41AM -0400, Mikulas Patocka wrote:
> > > On Tue, 18 Aug 2015, Mike Snitzer wrote:
> > > > On Tue, Aug 18 2015 at  4:26pm -0400,
> > > > Mikulas Patocka <mpatocka@redhat.com> wrote:
> > > > > Mike, please submit this patch to the upstream kernel before 4.2 is 
> > > > > released, so that the all the kernels that support the options 
> > > > > "precise_timestamps" and "histogram" also report these values back.
> > > > 
> > > > Userspace will have to check the DM ioctl version to know if DM provides
> > > > these extra keywords.
> > > > 
> > > > Mike
> > > 
> > > But if you don't send it, userspace will forever have to deal with kernels 
> > > that don't report this feature back.
> > 
> > We'd either end up saying some kernels can't support it when they can, or
> > have to carry some additional userspace clutter forever to provide a 
> > workaround.
> > 
> > Regards,
> > Bryn.
> 
> I think you don't need to provide a workaround for kernels that don't 
> report it. The workaround would be needed just for 4.2.0 (Mike said he 
> will backport the fix to 4.2.1), there is no need to bloat userspace with 
> that.

That's what's been done; the aux data stuff has been stripped out & I've
set the ioctl version check at 4.33.0.

Bryn.