Re: [PATCH v6 0/2] tools/nolibc: fix up size inflat regression

[Zhangjin Wu]

Hi, Willy

> On Sat, Aug 12, 2023 at 05:49:36AM +0800, Zhangjin Wu wrote:
> > After these two patches, will send the proposed my_syscall() patchset
> > tomorrow, it can even further reduce more type conversions and therefore
> > reduce more binary bytes, here is a preview of the testing result:
> > 
> >    // with the coming my_syscall() patchset, sys_* from functionsn to macros
> >      i386: 160 test(s): 158 passed,   2 skipped,   0 failed => status: warning 19250
> >    x86_64: 160 test(s): 158 passed,   2 skipped,   0 failed => status: warning 21733
> (...)
> > It can also shrink the whole sys.h from 1171 lines to around 738 lines.
> 
> Please, Zhangjin, please. Let's stop constantly speaking about potential
> future improvements when the present is broken. It needlessly adds a lot
> of noise in the discussion and tends to encourage you to explore areas
> that are incompatible with what is required to fix the breakage, and
> very likely steers your approach to fixes in a direction that you think
> is compatible with such future paths. But as long as existing code is
> broken you cannot speculate on how better the next iteration will be,
> because it's built on a broken basis. And I would like to remind that
> the *only* reason for the current breakage is this attempt to save even
> more code lines, that was not a requirement at all in the first place!
> Sure it can be fine to remove code when possible, but not at the cost of
> trying to force squares to enter round holes like this. The reality is
> that *some* syscalls are different and *some* archs are different, and
> these differences have to be taken into account, and if we keep exceptions
> it's fine.
>

Agree very much, that's why I didn't send the new patchset but only send
these two ones about size inflate regression, I don't want to discuss
more than one issue at a time either (and you also have shared this idea
several times) ;-)

The progress and preview data here is only because the patch 1/2 [1] is
an important preparation of the new patchset, the data here is more or
less providing a selling point why we need patch 1/2, I have explained
it in this reply [2]. Of course, we can roll them back directly, and If
we do need sys_brk/mmap return 'long', we can revert the rolling-back
and apply patch 1/2.

    [PATCH v6 1/2] tools/nolibc: let sys_brk, sys_mmap and sys_mmap2 return long

> So let's only speak about this later once the issue is completely solved.
>

Ok, it is the right direction.

Best regards,
Zhangjin
---
[1]: https://lore.kernel.org/lkml/82b584cbda5cee8d5318986644a2a64ba749a098.1691788036.git.falcon@xxxxxxxxxxx/
[2]: https://lore.kernel.org/lkml/20230813132620.19411-1-falcon@xxxxxxxxxxx/

> Thanks,
> Willy