[v4,01/11] cpumask: factor out show_cpumap into separate helper function

Commit Message

Sudeep Holla Sept. 4, 2014, 10:43 a.m. UTC

Hi Peter,

On 04/09/14 10:21, Peter Zijlstra wrote:
> On Thu, Sep 04, 2014 at 10:03:25AM +0100, Sudeep Holla wrote:
>> If I don't, every file including this header generates "warning:
>> ‘cpumap_copy_to_buf’ defined but not used". Alternatively I can just
>> declare here and define it elsewhere but I could not find more apt
>> place than this. Any suggestions ? kernel/cpu.c ?
>
> Sure the tail of cpu.c seems to have various other cpumask gunk in
> already so it fits there. The other alternative would've been
> lib/bitmap.c since that's where the various bitmap printf
> implementations live.
>
> You could make it bitmap_copy_to_buf() I suppose, and then have a
> cpumask inline wrapper, to be fully consistent with the rest of the
> implementations.
>

I liked the second approach, how about patch below ?
If will munge it into original patch.

--->8

    * __bitmap_parselist - convert list format ASCII string to bitmap
    * @buf: read nul-terminated user string from this buffer
    * @buflen: buffer size in bytes.  If string is smaller than this

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Peter Zijlstra Sept. 4, 2014, 11:25 a.m. UTC | #1

On Thu, Sep 04, 2014 at 11:43:18AM +0100, Sudeep Holla wrote:
> >You could make it bitmap_copy_to_buf() I suppose, and then have a
> >cpumask inline wrapper, to be fully consistent with the rest of the
> >implementations.
> >
> 
> I liked the second approach, how about patch below ?
> If will munge it into original patch.

Looks good, one more nit if you will permit :-)

> +int bitmap_copy_to_pagebuf(bool list, const unsigned long *maskp, char *buf)

Maybe s/copy/print/ or something? Its not really a 'copy' as such since
it very much transforms the bitmap.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Sudeep Holla Sept. 4, 2014, 12:27 p.m. UTC | #2

On 04/09/14 12:25, Peter Zijlstra wrote:
> On Thu, Sep 04, 2014 at 11:43:18AM +0100, Sudeep Holla wrote:
>>> You could make it bitmap_copy_to_buf() I suppose, and then have a
>>> cpumask inline wrapper, to be fully consistent with the rest of the
>>> implementations.
>>>
>>
>> I liked the second approach, how about patch below ?
>> If will munge it into original patch.
>
> Looks good, one more nit if you will permit :-)
>

Ofcourse :)

>> +int bitmap_copy_to_pagebuf(bool list, const unsigned long *maskp, char *buf)
>
> Maybe s/copy/print/ or something? Its not really a 'copy' as such since
> it very much transforms the bitmap.
>

Makes sense, will update accordingly.

Regards,
Sudeep

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index e1c8d080c427..34586d386c1f 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -60,6 +60,7 @@ 
    * bitmap_find_free_region(bitmap, bits, order)        Find and 
allocate bit region
    * bitmap_release_region(bitmap, pos, order)   Free specified bit region
    * bitmap_allocate_region(bitmap, pos, order)  Allocate specified bit 
region
+ * bitmap_copy_to_pagebuf(list, src, buf)      Print bitmap src as 
list/hex to buf
    */

   /*
@@ -145,6 +146,8 @@  extern void bitmap_release_region(unsigned long 
*bitmap, unsigned int pos, int o
   extern int bitmap_allocate_region(unsigned long *bitmap, unsigned int 
pos, int order);
   extern void bitmap_copy_le(void *dst, const unsigned long *src, int 
nbits);
   extern int bitmap_ord_to_pos(const unsigned long *bitmap, int n, int 
bits);
+extern int bitmap_copy_to_pagebuf(bool list, const unsigned long *maskp,
+                                 char *buf);

   #define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) % BITS_PER_LONG))
   #define BITMAP_LAST_WORD_MASK(nbits)                                   \
diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index 26d8348292dd..5b3bd163c5a0 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -807,16 +807,7 @@  static inline const struct cpumask 
*get_cpu_mask(unsigned int cpu)
   static inline ssize_t
   cpumap_copy_to_buf(bool list, const struct cpumask *mask, char *buf)
   {
-       ptrdiff_t len = PTR_ALIGN(buf + PAGE_SIZE - 1, PAGE_SIZE) - buf - 2;
-       int n = 0;
-
-       if (len > 1) {
-               n = list ? cpulist_scnprintf(buf, len, mask) :
-                          cpumask_scnprintf(buf, len, mask);
-               buf[n++] = '\n';
-               buf[n] = '\0';
-       }
-       return n;
+       return bitmap_copy_to_pagebuf(list, cpumask_bits(mask), buf);
   }

sudeep@e103737-lin:~/work/power/kernel$ git diff > /tmp/log
sudeep@e103737-lin:~/work/power/kernel$ cat /tmp/log
diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index e1c8d080c427..34586d386c1f 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -60,6 +60,7 @@ 
    * bitmap_find_free_region(bitmap, bits, order)	Find and allocate bit 
region
    * bitmap_release_region(bitmap, pos, order)	Free specified bit region
    * bitmap_allocate_region(bitmap, pos, order)	Allocate specified bit 
region
+ * bitmap_copy_to_pagebuf(list, src, buf)	Print bitmap src as list/hex 
to buf
    */

   /*
@@ -145,6 +146,8 @@  extern void bitmap_release_region(unsigned long 
*bitmap, unsigned int pos, int o
   extern int bitmap_allocate_region(unsigned long *bitmap, unsigned int 
pos, int order);
   extern void bitmap_copy_le(void *dst, const unsigned long *src, int 
nbits);
   extern int bitmap_ord_to_pos(const unsigned long *bitmap, int n, int 
bits);
+extern int bitmap_copy_to_pagebuf(bool list, const unsigned long *maskp,
+				  char *buf);

   #define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) % BITS_PER_LONG))
   #define BITMAP_LAST_WORD_MASK(nbits)					\
diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index 26d8348292dd..5b3bd163c5a0 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -807,16 +807,7 @@  static inline const struct cpumask 
*get_cpu_mask(unsigned int cpu)
   static inline ssize_t
   cpumap_copy_to_buf(bool list, const struct cpumask *mask, char *buf)
   {
-	ptrdiff_t len = PTR_ALIGN(buf + PAGE_SIZE - 1, PAGE_SIZE) - buf - 2;
-	int n = 0;
-
-	if (len > 1) {
-		n = list ? cpulist_scnprintf(buf, len, mask) :
-			   cpumask_scnprintf(buf, len, mask);
-		buf[n++] = '\n';
-		buf[n] = '\0';
-	}
-	return n;
+	return bitmap_copy_to_pagebuf(list, cpumask_bits(mask), buf);
   }

   /*
diff --git a/lib/bitmap.c b/lib/bitmap.c
index 1e031f2c9aba..b23adef04d7e 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -580,6 +580,32 @@  int bitmap_scnlistprintf(char *buf, unsigned int 
buflen,
   EXPORT_SYMBOL(bitmap_scnlistprintf);

   /**
+ * bitmap_copy_to_pagebuf - convert bitmap to list or hex format ASCII 
string
+ * @list: indicates whether the bitmap must be list
+ * @maskp: pointer to bitmap to convert
+ * @buf: page aligned buffer into which string is placed
+ *
+ * Output format is a comma-separated list of decimal numbers and
+ * ranges if list is specified or hex digits grouped into comma-separated
+ * sets of 8 digits/set. Returns the number of characters written to buf.
+ */
+int bitmap_copy_to_pagebuf(bool list, const unsigned long *maskp, char 
*buf)
+{
+	ptrdiff_t len = PTR_ALIGN(buf + PAGE_SIZE - 1, PAGE_SIZE) - buf - 2;
+	int n = 0;
+
+	if (len > 1) {
+		n = list ?
+		    bitmap_scnlistprintf(buf, len, maskp, nr_cpumask_bits) :
+		    bitmap_scnprintf(buf, len, maskp, nr_cpumask_bits);
+		buf[n++] = '\n';
+		buf[n] = '\0';
+	}
+	return n;
+}
+EXPORT_SYMBOL(bitmap_copy_to_pagebuf);
+
+/**