From: Rich Felker <email@example.com>
To: 王洪亮 <firstname.lastname@example.org>
Subject: Re: [musl] add loongarch64 port v6.
Date: Thu, 16 Feb 2023 18:13:37 -0500 [thread overview]
Message-ID: <20230216231336.GW4163@brightrain.aerifal.cx> (raw)
On Mon, Jan 30, 2023 at 09:27:21AM +0800, 王洪亮 wrote:
> 在 2023/1/30 上午1:04, Rich Felker 写道:
> >On Sun, Jan 29, 2023 at 03:52:18AM -0500, Ariadne Conill wrote:
> >>On 2023-01-28 20:15, 王洪亮 wrote:
> >>>Is there anything else that needs to be modified in patch v6?
> >>It is likely that you will get a better review if you split up
> >>the changes into logical ones and submit them to the list as
> >>a group of patches.
> >It's been a while since I looked in detail, but I don't think that's
> >necessary here. It should just be a matter of ensuring that all
> >previously requested changes were made, and that the ABI is
> >official/stable on kernel and compiler sides. I've been away overseas
> >(on vacation) for the past month and this kind of review is outside
> >the scope of what I've been checking in on while away, but I will be
> >back in roughly a week.
> Yes,this patch is modified according to the previous suggestions,
> and the ABI is consistent with the kernel and compiler sides.
> I'm looking forward to your review and reply in a week.thanks.
One thing that's come up since previous review is that we had things
wrong around the kernel sigaction ABI on a number of archs. From the
way you defined SA_RESTORER as 0, it looks like loongarch64 is
intended not to have a restorer member in the kernel sigaction
structure. Can you confirm? I think this means the ksigaction you're
using in the musl port right now is wrong and mismatched with the
kernel. If my understanding is right, once my patches for fixing the
other archs are pushed, just removing the #define SA_RESTORER 0 line
will make this correct.
As long as the kernel has officially decided on adopting __NR_clone,
it's fine (and preferable) to stick with using it for __clone.
I still see a few places with whitespace issues here and there but I
don't want to waste your time with them; I can clean them up in the
diff before applying it.
I also spotted some minor namespace details in bits/signal.h. I don't
think this needs to block merge. I can prepare/propose a patch on top
of the one adding the arch.
So, really I think the only thing I need right now is to know whether
my understanding of the SA_RESTORER situation is correct.
next prev parent reply other threads:[~2023-02-16 23:13 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-03 8:15 [musl] add loongarch64 port v5 王洪亮
2022-09-06 1:51 ` [musl] " 王洪亮
2022-09-06 5:12 ` WANG Xuerui
2022-10-08 1:44 ` 王洪亮
2022-10-12 8:22 ` [musl] " WANG Xuerui
2022-11-15 7:17 ` [musl] add loongarch64 port v6 王洪亮
2022-12-19 6:32 ` 王洪亮
2023-01-09 9:46 ` 王洪亮
2023-01-29 1:15 ` 王洪亮
2023-01-29 8:52 ` Ariadne Conill
2023-01-29 17:04 ` Rich Felker
2023-01-30 1:27 ` 王洪亮
2023-02-16 23:13 ` Rich Felker [this message]
2023-02-17 7:06 ` 王洪亮
2023-03-17 8:41 ` 王洪亮
2023-04-03 13:35 ` Szabolcs Nagy
2023-04-03 14:42 ` Rich Felker
2023-04-03 15:44 ` Szabolcs Nagy
2023-04-04 9:46 ` 王洪亮
2023-04-04 17:06 ` Szabolcs Nagy
2023-04-11 10:00 ` 王洪亮
2023-04-12 2:21 ` 王洪亮
2022-10-12 18:20 ` [musl] add loongarch64 port v5 Rich Felker
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:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
* 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.
Code repositories for project(s) associated with this public inbox
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).