From: Gao Xiang <gaoxiang25@huawei.com>
To: Amir Goldstein <amir73il@gmail.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Andrew Morton <akpm@linux-foundation.org>,
Stephen Rothwell <sfr@canb.auug.org.au>,
Theodore Ts'o <tytso@mit.edu>,
"Linus Torvalds" <torvalds@linux-foundation.org>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
<devel@driverdev.osuosl.org>, LKML <linux-kernel@vger.kernel.org>,
<linux-erofs@lists.ozlabs.org>, Chao Yu <yuchao0@huawei.com>,
Miao Xie <miaoxie@huawei.com>,
Li Guifu <bluce.liguifu@huawei.com>,
Fang Wei <fangwei1@huawei.com>,
Steven Rostedt <rostedt@goodmis.org>,
Ingo Molnar <mingo@redhat.com>,
Matthew Wilcox <willy@infradead.org>,
Will Deacon <will@kernel.org>,
Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH v3 12/24] erofs: introduce tagged pointer
Date: Mon, 22 Jul 2019 14:31:00 +0800 [thread overview]
Message-ID: <a9d4920c-cfb0-50c7-a1f3-3e3c0e581bd7@huawei.com> (raw)
In-Reply-To: <CAOQ4uxgo5kvgoEn7SbuwF9+B1W9Qg1-2jSUm5+iKZdT6-wDEog@mail.gmail.com>
On 2019/7/22 14:16, Amir Goldstein wrote:
> On Mon, Jul 22, 2019 at 8:02 AM Gao Xiang <gaoxiang25@huawei.com> wrote:
>>
>> Hi Amir,
>>
>> On 2019/7/22 12:39, Amir Goldstein wrote:
>>> On Mon, Jul 22, 2019 at 5:54 AM Gao Xiang <gaoxiang25@huawei.com> wrote:
>>>>
>>>> Currently kernel has scattered tagged pointer usages
>>>> hacked by hand in plain code, without a unique and
>>>> portable functionset to highlight the tagged pointer
>>>> itself and wrap these hacked code in order to clean up
>>>> all over meaningless magic masks.
>>>>
>>>> This patch introduces simple generic methods to fold
>>>> tags into a pointer integer. Currently it supports
>>>> the last n bits of the pointer for tags, which can be
>>>> selected by users.
>>>>
>>>> In addition, it will also be used for the upcoming EROFS
>>>> filesystem, which heavily uses tagged pointer pproach
>>>> to reduce extra memory allocation.
>>>>
>>>> Link: https://en.wikipedia.org/wiki/Tagged_pointer
>>>
>>> Well, it won't do much good for other kernel users in fs/erofs/ ;-)
>>
>> Thanks for your reply and interest in this patch.... :)
>>
>> Sigh... since I'm not sure kernel folks could have some interests in that stuffs.
>>
>> Actually at the time once I coded EROFS I found tagged pointer had 2 main advantages:
>> 1) it saves an extra field;
>> 2) it can keep the whole stuff atomicly...
>> And I observed the current kernel uses tagged pointer all around but w/o a proper wrapper...
>> and EROFS heavily uses tagged pointer... So I made a simple tagged pointer wrapper
>> to avoid meaningless magic masks and type casts in the code...
>>
>>>
>>> I think now would be a right time to promote this facility to
>>> include/linux as you initially proposed.
>>> I don't recall you got any objections. No ACKs either, but I think
>>> that was the good kind of silence (?)
>>
>> Yes, no NAK no ACK...(it seems the ordinary state for all EROFS stuffs... :'( sigh...)
>> Therefore I decided to leave it in fs/erofs/ in this series...
>>
>>>
>>> You might want to post the __fdget conversion patch [1] as a
>>> bonus patch on top of your series.
>>
>> I am not sure if another potential users could be quite happy with my ("sane?" or not)
>> implementation...
>
> Well, let's ask potential users then.
>
> CC kernel/trace maintainers for RB_PAGE_HEAD/RB_PAGE_UPDATE
> and kernel/locking maintainers for RT_MUTEX_HAS_WAITERS
>
>> (Is there some use scenerios in overlayfs and fanotify?...)
>
> We had one in overlayfs once. It is gone now.
>
>>
>> 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.
>
> Also pointing out that fixing potential bugs in one implementation is preferred
> to having to patch all copied implementations.
>
> I wonder if tagptr_unfold_tags() doesn't need READ_ONCE() as per:
> 1be5d4fa0af3 locking/rtmutex: Use READ_ONCE() in rt_mutex_owner()
>
> rb_list_head() doesn't have READ_ONCE()
> Nor does hlist_bl_first() and BPF_MAP_PTR().
>
> Are those all safe due to safe call sites? or potentially broken?
...Add a word (maybe not too ralated with this topic), I heard something
before from compiler guys like that the pointer type will be kept in atomic
by compilers during accessing, I personally think that makes sense
for pointer type.
However, in EROFS implementation (not in this patch) I tend to use
WRITE_ONCE / READ_ONCE in order to access once and as a hint to tell
compiler it should be access once in case of getting rare broken
generated code...
I cannot trust compiler all the time due to code optimization since
1) I have no idea it will generate in atomic for all cases...
2) I have no idea it will be accessed more than one time somewhere...
Thanks,
Gao Xiang
>
> Thanks,
> Amir.
>
next prev parent reply other threads:[~2019-07-22 6:31 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-22 2:50 [PATCH v3 00/24] erofs: promote erofs from staging Gao Xiang
2019-07-22 2:50 ` [PATCH v3 01/24] erofs: add on-disk layout Gao Xiang
2019-07-22 3:26 ` Stephen Rothwell
2019-07-22 3:29 ` Gao Xiang
2019-07-22 5:05 ` Greg Kroah-Hartman
2019-07-22 5:24 ` Gao Xiang
2019-07-22 2:50 ` [PATCH v3 02/24] erofs: add erofs in-memory stuffs Gao Xiang
2019-07-22 2:50 ` [PATCH v3 03/24] erofs: add super block operations Gao Xiang
2019-07-22 2:50 ` [PATCH v3 04/24] erofs: add raw address_space operations Gao Xiang
2019-07-22 2:50 ` [PATCH v3 05/24] erofs: add inode operations Gao Xiang
2019-07-22 2:50 ` [PATCH v3 06/24] erofs: support special inode Gao Xiang
2019-07-22 2:50 ` [PATCH v3 07/24] erofs: add directory operations Gao Xiang
2019-07-22 2:50 ` [PATCH v3 08/24] erofs: add namei functions Gao Xiang
2019-07-22 2:50 ` [PATCH v3 09/24] erofs: support tracepoint Gao Xiang
2019-07-22 2:50 ` [PATCH v3 10/24] erofs: update Kconfig and Makefile Gao Xiang
2019-07-22 2:50 ` [PATCH v3 11/24] erofs: introduce xattr & posixacl support Gao Xiang
2019-07-22 2:50 ` [PATCH v3 12/24] erofs: introduce tagged pointer Gao Xiang
2019-07-22 4:39 ` Amir Goldstein
2019-07-22 5:01 ` Gao Xiang
2019-07-22 6:16 ` Amir Goldstein
2019-07-22 6:31 ` Gao Xiang [this message]
2019-07-22 14:40 ` Steven Rostedt
2019-07-22 15:33 ` Gao Xiang
2019-07-22 16:35 ` Steven Rostedt
2019-07-22 16:52 ` Gao Xiang
2019-07-22 2:50 ` [PATCH v3 13/24] erofs: add compression indexes support Gao Xiang
2019-07-22 2:50 ` [PATCH v3 14/24] erofs: introduce superblock registration Gao Xiang
2019-07-22 2:50 ` [PATCH v3 15/24] erofs: introduce erofs shrinker Gao Xiang
2019-07-22 2:50 ` [PATCH v3 16/24] erofs: introduce workstation for decompression Gao Xiang
2019-07-22 2:50 ` [PATCH v3 17/24] erofs: introduce per-CPU buffers implementation Gao Xiang
2019-07-22 2:50 ` [PATCH v3 18/24] erofs: introduce pagevec for decompression subsystem Gao Xiang
2019-07-22 2:50 ` [PATCH v3 19/24] erofs: add erofs_allocpage() Gao Xiang
2019-07-22 2:50 ` [PATCH v3 20/24] erofs: introduce generic decompression backend Gao Xiang
2019-07-22 2:50 ` [PATCH v3 21/24] erofs: introduce LZ4 decompression inplace Gao Xiang
2019-07-22 2:50 ` [PATCH v3 22/24] erofs: introduce the decompression frontend Gao Xiang
2019-07-22 2:50 ` [PATCH v3 23/24] erofs: introduce cached decompression Gao Xiang
2019-07-22 10:18 ` David Sterba
2019-07-22 10:58 ` Gao Xiang
2019-07-22 13:25 ` Theodore Y. Ts'o
2019-07-22 14:16 ` Gao Xiang
2019-07-22 15:12 ` Theodore Y. Ts'o
2019-07-22 16:27 ` Gao Xiang
2019-07-23 12:31 ` David Sterba
2019-07-23 13:30 ` Gao Xiang
2019-07-22 2:50 ` [PATCH v3 24/24] erofs: add document Gao Xiang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a9d4920c-cfb0-50c7-a1f3-3e3c0e581bd7@huawei.com \
--to=gaoxiang25@huawei.com \
--cc=akpm@linux-foundation.org \
--cc=amir73il@gmail.com \
--cc=bluce.liguifu@huawei.com \
--cc=devel@driverdev.osuosl.org \
--cc=fangwei1@huawei.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-erofs@lists.ozlabs.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=miaoxie@huawei.com \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=sfr@canb.auug.org.au \
--cc=torvalds@linux-foundation.org \
--cc=tytso@mit.edu \
--cc=viro@zeniv.linux.org.uk \
--cc=will@kernel.org \
--cc=willy@infradead.org \
--cc=yuchao0@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).