Date: Fri, 17 Mar 2023 16:41:26 +0800 From: 王洪亮 <wanghongliang@...ngson.cn> To: musl@...ts.openwall.com Subject: Re: add loongarch64 port v6. 在 2023/2/17 下午3:06, 王洪亮 写道: > > 在 2023/2/17 上午7:13, Rich Felker 写道: >> 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: >>>>> Hi, >>>>> >>>>> On 2023-01-28 20:15, 王洪亮 wrote: >>>>>> Hi, >>>>>> >>>>>> 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. >>>> >>>> Rich >>> 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. >> >> Rich > Yes, your understanding is correct. I have confirmed that there is no > SA_RESTORER > define in loongarch64, so no sa_restorer member in kernel sigaction. > > Hongliang Wang Hi, Rich Do I need to do any other work with this patch v6？ Hongliang Wang
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.