linux-kernel - Re: [PATCH 1/9] exofs: osd Swiss army knife

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4961CC87.4030903@panasas.com>
Date:	Mon, 05 Jan 2009 11:01:59 +0200
From:	Boaz Harrosh <bharrosh@...asas.com>
To:	Pavel Machek <pavel@...e.cz>
CC:	Andrew Morton <akpm@...ux-foundation.org>, avishay@...il.com,
	jeff@...zik.org, viro@...IV.linux.org.uk,
	linux-fsdevel@...r.kernel.org, osd-dev@...n-osd.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/9] exofs: osd Swiss army knife

Pavel Machek wrote:
> On Sun 2009-01-04 10:43:09, Boaz Harrosh wrote:
>> Pavel Machek wrote:
>>> Hi!
>>>
>>>>> +#ifdef ARCH_HAS_ATOMIC_UNSIGNED
>>>> This doesn't exist, and it would be fairly bad to introduce it.  Please
>>>> kill the ifdefs.
>>>>
>>>>> +typedef unsigned exofs_iflags_t;
>>>>> +#else
>>>>> +typedef unsigned long exofs_iflags_t;
>>>>> +#endif
>>>> Then please kill the typedef altogether and replace it with `unsigned
>>>> long' everywhere
>>> Hmmm.. .and at a note somewhere that we assume unsigned long to be atomic...?
>>>
>> I think I'll just use unsigned. It's more then enough I'm not using more then 3
>> bits for now. Is unsigned workable for all ARCHs?

<added>
> /*
>  * our extension to the in-memory inode
>  */
> struct exofs_i_info {
> 	unsigned long  i_flags;            /* various atomic flags            */
<snip>
> 
> /*
>  * our inode flags
>  */
> #define OBJ_2BCREATED	0	/* object will be created soon*/
> #define OBJ_CREATED	1	/* object has been created on the osd*/
> 
> static inline int obj_2bcreated(struct exofs_i_info *oi)
> {
> 	return test_bit(OBJ_2BCREATED, &(oi->i_flags));
> }
> 
> static inline void set_obj_2bcreated(struct exofs_i_info *oi)
> {
> 	set_bit(OBJ_2BCREATED, &(oi->i_flags));
> }
> 
> static inline int obj_created(struct exofs_i_info *oi)
> {
> 	return test_bit(OBJ_CREATED, &(oi->i_flags));
> }
> 
> static inline void set_obj_created(struct exofs_i_info *oi)
> {
> 	set_bit(OBJ_CREATED, &(oi->i_flags));
> }
</added>

> 
> Please just use atomic_t.
> 
> (see "atomics: document that linux expects certain atomic behaviour"
> thread for discussion)
> 									Pavel

I have a problem with this. The context of i_flags is to be used with
set_bit() and test_bit(). In some ARCHs like x86_64 they take an 
"unsigned long *" in most others they take a "void *" and cast internally
to a "u32 *". (for x86_64 I must use "unsigned long", anything else warns)

I think if I declare "unsigned long" but only use 32 bits flags then
I should be in the clear with ALL archs, I'll see if that works once
this code sits in linux-next. (That's real ugly I think)

Is set_bit() and test_bit() should only be used from arch/ code? What 
can regular kernel code use?

Thanks
Boaz
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/