linux-kernel - Re: [PATCH v3 12/24] erofs: introduce tagged pointer

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 for Android: free password hash cracker in your pocket

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

Message-ID: <25e5dd99-b110-45f8-f769-5372ff01510f@aol.com>
Date:   Tue, 23 Jul 2019 00:52:14 +0800
From:   Gao Xiang <hsiangkao@....com>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     Amir Goldstein <amir73il@...il.com>, devel@...verdev.osuosl.org,
        Stephen Rothwell <sfr@...b.auug.org.au>,
        Matthew Wilcox <willy@...radead.org>,
        Theodore Ts'o <tytso@....edu>, Will Deacon <will@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Miao Xie <miaoxie@...wei.com>, linux-erofs@...ts.ozlabs.org,
        LKML <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...hat.com>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH v3 12/24] erofs: introduce tagged pointer



On 2019/7/23 ????12:35, Steven Rostedt wrote:
> On Mon, 22 Jul 2019 23:33:53 +0800
> Gao Xiang <hsiangkao@....com> wrote:
> 
>> Hi Steven,
>>
>> On 2019/7/22 ????10:40, Steven Rostedt wrote:
>>>>> and I'm not sure Al could accept __fdget conversion (I just wanted to give a example then...)
>>>>>
>>>>> Therefore, I tend to keep silence and just promote EROFS... some better ideas?...
>>>>>    
>>>> Writing example conversion patches to demonstrate cleaner code
>>>> and perhaps reduce LOC seems the best way.  
>>> Yes, I would be more interested in seeing patches that clean up the
>>> code than just talking about it.
>>>   
>>
>> I guess that is related to me, though I didn't plan to promote
>> a generic tagged pointer implementation in this series...
> 
> I don't expect you to either.

Beyond my expectation, I think I will (could) learn some new knowledge
from this topic, thanks you and Amir :)

> 
>>
>> I try to describe what erofs met and my own implementation,
>> assume that we have 3 tagged pointers, a, b, c, and one
>> potential user only (no need to ACCESS_ONCE).
>>
>> One way is
>>
>> #define A_MASK		1
>> #define B_MASK		1
>> #define C_MASK		3
>>
>> /* now we have 3 mask there, A, B, C is simple,
>>    the real name could be long... */
>>
>> void *a;
>> void *b;
>> void *c;		/* and some pointers */
>>
>> In order to decode the tag, we have to
>> 	((unsigned long)a & A_MASK)
>>
>> to decode the ptr, we have to
>> 	((unsigned long)a & ~A_MASK)
>>
>> In order to fold the tagged pointer...
>> 	(void *)((unsigned long)a | tag)
> 
> And you need a way to clear the flag.

Considering one potential user, we could refold the tagged pointer.
or we could refold the tagged pointer and update the value in atomic
(like atomic_t does).

a = tagptr_fold(ta, tagptr_unfold_tags(a), tag);

> 
>>
>> You can see the only meaning of these masks is the bitlength of tags,
>> but there are many masks (or we have to do open-coded a & 3,
>> if bitlength is changed, we have to fix them all)...
>>
>> therefore my approach is
>>
>> typedef tagptr1_t ta;	/* tagptr type a with 1-bit tag */
>> typedef tagptr1_t tb;	/* tagptr type b with 1-bit tag */
>> typedef tagptr2_t tc;	/* tagptr type c with 2-bit tag */
>>
>> and ta a; tb b; tc c;
>>
>> the type will represent its bitlength of tags and we can use ta, tb, tc
>> to avoid masks or open-coded bitlength.
>>
>> In order to decode the tag, we can
>> 	tagptr_unfold_tags(a)
>>
>> In order to decode the ptr, we can
>> 	tagptr_unfold_ptr(a)
>>
>> In order to fold the tagged pointer...
>> 	a = tagptr_fold(ta, ptr, tag)
>>
>>
>> ACCESS_ONCE stuff is another thing... If my approach seems cleaner,
>> we could move to include/linux later after EROFS stuffs is done...
>> Or I could use a better tagptr approach later if any...
> 
> Looking at the ring buffer code, it may be a bit too complex to try to
> use a generic infrastructure. Look at rb_head_page_set(), where it does
> a cmpxchg to set or clear the flags and then tests the previous flags
> to know what actions need to be done.

The current code supports cmpxchg as well, but I don't look into
rb_head_page_set... (although I think it is not the critical thing if we
decide to do some generic tagged pointer approach...)

> 
> The ring buffer tag code was added in 2009, the rtmutex tag code was
> added in 2006. It's been 10 years before we needed another tag
> operation. I'm not sure we benefit from making this generic.

Okay, that depends on your folks, actually...


Thanks,
Gao Xiang

> 
> -- Steve
>