On 2019/7/23 ????12:35, Steven Rostedt wrote: > On Mon, 22 Jul 2019 23:33:53 +0800 > Gao Xiang <hsiangkao@xxxxxxx> 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 >
