flight recorder: switch from int to bytes for requested allocation

The flight recoder buffer size as specified in LOGSYS_DECLARE_SYSTEM
or _logsys_rec_init was expressed in number of ints. A developer asking
to allocate 512K would get a 2M allocation on a machine with sizeof(int) = 4.

This is confusing and the patch addresses it:

- rename rec_size to fltsize for external API (no type change),
  because rec_size is used many times internally for other reasons
  and it can be confusing.

- rename size to fltsize in _logsys_rec_init.

- document what we allocate and why.

- swap comments around to match the code.

- introduce a simple macro to perform rounding (stolen from linux-2.6.git).

- start shaping fdata header to better handle dynamic values:
  * write the flt_data_size as first unsigned int the header.
  * change corosync-fplay to read the value and alloc the right amount
    of memory instead of hardcoding it again.



git-svn-id: http://svn.fedorahosted.org/svn/corosync/trunk@2255 fd59a12c-fef9-0310-b244-a6a79926bd2f

exec/logsys.c

@@ -66,6 +66,8 @@
 
 #define MIN(x,y) ((x) < (y) ? (x) : (y))
 
+#define ROUNDUP(x, y) ((((x) + ((y) - 1)) / (y)) * (y))
+
 /*
  * syslog prioritynames, facility names to value mapping
  * Some C libraries build this in to their headers, but it is non-portable
@@ -119,7 +121,7 @@ struct syslog_names facilitynames[] =
  */
 int *flt_data;
 
-int flt_data_size;
+unsigned int flt_data_size;
 
 #define COMBINE_BUFFER_SIZE 2048
 
@@ -1044,17 +1046,39 @@ int _logsys_wthread_create (void)
 	return (0);
 }
 
-int _logsys_rec_init (unsigned int size)
+int _logsys_rec_init (unsigned int fltsize)
 {
 	/*
-	 * First record starts at zero
-	 * Last record ends at zero
+	 * we need to allocate:
+	 * - requested size +
+	 *   2 extra unsigned ints for HEAD/TAIL tracking
+	 *
+	 * then round it up to the next PAGESIZE
 	 */
-	flt_data = malloc ((size + 2) * sizeof (unsigned int));
+	size_t flt_real_size;
+
+	flt_real_size = ROUNDUP(
+			(fltsize + (2 * sizeof (unsigned int))),
+			sysconf(_SC_PAGESIZE));
+
+	flt_data = malloc (flt_real_size);
 	if (flt_data == NULL) {
 		return (-1);
 	}
-	flt_data_size = size;
+
+	/*
+	 * flt_data_size tracks data by ints and not bytes/chars.
+	 *
+	 * the last 2 ints are reserved to store HEAD/TAIL information.
+	 * hide them from the rotating buffer.
+	 */
+
+	flt_data_size = ((flt_real_size / sizeof (unsigned int)) - 2);
+
+	/*
+	 * First record starts at zero
+	 * Last record ends at zero
+	 */
 	flt_data[FDHEAD_INDEX] = 0;
 	flt_data[FDTAIL_INDEX] = 0;
 
@@ -1595,6 +1619,12 @@ int logsys_log_rec_store (const char *filename)
 		return (-1);
 	}
 
+	written_size = write (fd, &flt_data_size, sizeof(unsigned int));
+	if ((written_size < 0) || (written_size != sizeof(unsigned int))) {
+		close (fd);
+		return (-1);
+	}
+
 	written_size = write (fd, flt_data, size_to_write);
 	if (close (fd) != 0)
 		return (-1);

include/corosync/engine/logsys.h

@@ -305,7 +305,7 @@ extern void *logsys_rec_end;
 #define LOGSYS_REC_END (&logsys_rec_end)
 
 #define LOGSYS_DECLARE_SYSTEM(name,mode,debug,file,file_priority,	\
-		syslog_facility,syslog_priority,format,rec_size)	\
+		syslog_facility,syslog_priority,format,fltsize)		\
 __attribute__ ((constructor))						\
 static void logsys_system_init (void)					\
 {									\
@@ -322,7 +322,7 @@ static void logsys_system_init (void)					\
 		exit (-1);						\
 	}								\
 									\
-	if (_logsys_rec_init (rec_size) < 0) {				\
+	if (_logsys_rec_init (fltsize) < 0) {				\
 		fprintf (stderr,					\
 			"Unable to initialize log flight recorder.\n");	\
 		exit (-1);						\

man/logsys_overview.8

@@ -62,7 +62,7 @@ require it and enables full logging capabilities before any application code is
 executed.
 
 #define LOGSYS_DECLARE_SYSTEM (name, mode, debug, file, file_priority,
-syslog_facility, syslog_priority, format, rec_size)
+syslog_facility, syslog_priority, format, fltsize)
 
 The name parameter is the name of the application or system.
 
@@ -99,7 +99,8 @@ syslog.
 The format parameter allows to set custom output format.
 Set to NULL to use built-in default.
 
-The rec_size parameter specifies the flight recorder buffer size.
+The fltsize parameter specifies the flight recorder buffer size in bytes. The requested value
+is increased by the size of 2 unsigned ints and rounded to the next PAGE_SIZE.
 
 An example declaration would be:
 

tools/corosync-fplay.c

@@ -18,7 +18,7 @@
 
 #include <corosync/engine/logsys.h>
 
-unsigned int flt_data_size = 1000000;
+unsigned int flt_data_size;
 
 unsigned int *flt_data;
 #define FDHEAD_INDEX		(flt_data_size)
@@ -464,14 +464,23 @@ int main (void)
 	int record_count = 1;
 	ssize_t n_read;
 	const char *data_file = LOCALSTATEDIR "/lib/corosync/fdata";
+	size_t n_required;
 
-	size_t n_required = (flt_data_size + 2) * sizeof (unsigned int);
 	if ((fd = open (data_file, O_RDONLY)) < 0) {
 		fprintf (stderr, "failed to open %s: %s\n",
 			 data_file, strerror (errno));
 		return EXIT_FAILURE;
 	}
 
+	n_required = sizeof (unsigned int);
+	n_read = read (fd, &flt_data_size, n_required);
+	if (n_read != n_required) {
+		fprintf (stderr, "Unable to read fdata header\n");
+		return EXIT_FAILURE;
+	}
+
+	n_required = ((flt_data_size + 2) * sizeof(unsigned int));
+
 	if ((flt_data = malloc (n_required)) == NULL) {
 		fprintf (stderr, "exhausted virtual memory\n");
 		return EXIT_FAILURE;