On 07/18/2013 09:42 PM, Christoph Lameter wrote:
> On Thu, 18 Jul 2013, Chen Gang wrote:
>
>> On 07/17/2013 10:46 PM, Christoph Lameter wrote:
>>> On Tue, 16 Jul 2013, Chen Gang wrote:
>>>
>>>> If we really use 32-bit as unsigned number, better to use 'U' instead of
>>>> 'UL' (e.g. 0x80000000U instead of 0x80000000UL).
>>>>
>>>> Since it is unsigned 32-bit number, it is better to use 'unsigned int'
>>>> instead of 'int', which can avoid related warnings if "EXTRA_CFLAGS=-W".
>>>
>>> Ok could you go through the kernel source and change that?
>>>
>>
>> Yeah, thanks, I should do it.
>>
>> Hmm... for each case of this issue, it need communicate with (review by)
>> various related maintainers.
>>
>> So, I think one patch for one variable (and related macro contents) is
>> enough.
>>
>> Is it OK ?
>
> The fundamental issue is that typically ints are used for flags and I
> would like to keep it that way. Changing the constants in slab.h and the
> allocator code to be unsigned int instead of unsigned long wont be that
> much of a deal.
>
At least, we need use 'unsigned' instead of 'signed'.
e.g.
----------------------------diff begin---------------------------------
diff --git a/mm/slub.c b/mm/slub.c
index 2b02d66..7111d7a 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -452,9 +452,9 @@ static void get_map(struct kmem_cache *s, struct page *page, unsigned long *map)
* Debug settings:
*/
#ifdef CONFIG_SLUB_DEBUG_ON
-static int slub_debug = DEBUG_DEFAULT_FLAGS;
+static unsigned int slub_debug = DEBUG_DEFAULT_FLAGS;
#else
-static int slub_debug;
+static unsigned int slub_debug;
#endif
static char *slub_debug_slabs;
----------------------------diff end-----------------------------------
> Will the code then be clean enough for you?
>
Hmm... Things maybe seem more complex, please see bellow:
For 'SLAB_RED_ZONE' (or the other constants), they also can be assigned
to "struct kmem_cache" member variable 'flags'.
But for "struct kmem_cache", it has 2 different definitions, they share
with the 'SLAB_RED_ZONE' (or the other constants).
One defines 'flags' as 'unsigned int' in "include/linux/slab_def.h"
16 /*
17 * struct kmem_cache
18 *
19 * manages a cache.
20 */
21
22 struct kmem_cache {
23 /* 1) Cache tunables. Protected by cache_chain_mutex */
24 unsigned int batchcount;
25 unsigned int limit;
26 unsigned int shared;
27
28 unsigned int size;
29 u32 reciprocal_buffer_size;
30 /* 2) touched by every alloc & free from the backend */
31
32 unsigned int flags; /* constant flags */
33 unsigned int num; /* # of objs per slab */
...
The other defines 'flags' as 'unsigned long' in "include/linux/slub_def.h"
(but from its comments, it even says it is for 'Slab' cache management !!)
65 /*
66 * Slab cache management.
67 */
68 struct kmem_cache {
69 struct kmem_cache_cpu __percpu *cpu_slab;
70 /* Used for retriving partial slabs etc */
71 unsigned long flags;
72 unsigned long min_partial;
73 int size; /* The size of an object including meta data */
74 int object_size; /* The size of an object without meta data */
75 int offset; /* Free pointer offset. */
76 int cpu_partial; /* Number of per cpu partial objects to keep around */
77 struct kmem_cache_order_objects oo;
...
Maybe it is also related with our discussion ('unsigned int' or 'unsigned long') ?
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@xxxxxxxxx. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
>
--
Chen Gang
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>