public inbox for [email protected]
 help / color / mirror / Atom feed
* [PATCH liburing 0/2] Changes for src/Makefile
@ 2022-03-08 22:40 Alviro Iskandar Setiawan
  2022-03-08 22:40 ` [PATCH liburing 1/2] src/Makefile: Remove `-fomit-frame-pointer` from default build Alviro Iskandar Setiawan
  2022-03-08 22:40 ` [PATCH liburing 2/2] src/Makefile: Add header files as dependency Alviro Iskandar Setiawan
  0 siblings, 2 replies; 9+ messages in thread
From: Alviro Iskandar Setiawan @ 2022-03-08 22:40 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Alviro Iskandar Setiawan, Pavel Begunkov, Ammar Faizi,
	Alviro Iskandar Setiawan, io-uring, gwml

Hello sir,

This patchset changes src/Makefile. 2 patches here:

1. Remove -fomit-frame-pointer flag, because it's already covered
   by the -O2 optimization flag.

2. Add file dependencies for the object files. When the header
   files are modified, the compiled object are not going to be
   recompiled because the header files are not marked as dependency
   for the objects. Add those header files as dependency so it is
   safe not to do "make clean" after changing those files.

please review,
thx

Signed-off-by: Alviro Iskandar Setiawan <[email protected]>
---
Alviro Iskandar Setiawan (2):
  src/Makefile: Remove `-fomit-frame-pointer` from default build
  src/Makefile: Add header files as dependency

 src/Makefile | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)


base-commit: 6231f56da7881bde6fb011e1b54d672f8fe5a224
-- 
2.32.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH liburing 1/2] src/Makefile: Remove `-fomit-frame-pointer` from default build
  2022-03-08 22:40 [PATCH liburing 0/2] Changes for src/Makefile Alviro Iskandar Setiawan
@ 2022-03-08 22:40 ` Alviro Iskandar Setiawan
  2022-03-08 22:40 ` [PATCH liburing 2/2] src/Makefile: Add header files as dependency Alviro Iskandar Setiawan
  1 sibling, 0 replies; 9+ messages in thread
From: Alviro Iskandar Setiawan @ 2022-03-08 22:40 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Alviro Iskandar Setiawan, Pavel Begunkov, Ammar Faizi,
	Alviro Iskandar Setiawan, io-uring, gwml

-fomit-frame-pointer is already turned on by -O1 optimization flag.

The liburing default compilation uses -O2 optimization, -O2 turns on
all optimization flags specified by -O1. Therefore, we don't need to
specify -fomit-frame-pointer here. Remove it.

Link: https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html
Signed-off-by: Alviro Iskandar Setiawan <[email protected]>
---
 src/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Makefile b/src/Makefile
index 3e1192f..f19d45e 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -8,7 +8,7 @@ libdevdir ?= $(prefix)/lib
 CPPFLAGS ?=
 override CPPFLAGS += -D_GNU_SOURCE \
 	-Iinclude/ -include ../config-host.h
-CFLAGS ?= -g -fomit-frame-pointer -O2 -Wall -Wextra -fno-stack-protector
+CFLAGS ?= -g -O2 -Wall -Wextra -fno-stack-protector
 override CFLAGS += -Wno-unused-parameter -Wno-sign-compare -DLIBURING_INTERNAL
 SO_CFLAGS=-fPIC $(CFLAGS)
 L_CFLAGS=$(CFLAGS)
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH liburing 2/2] src/Makefile: Add header files as dependency
  2022-03-08 22:40 [PATCH liburing 0/2] Changes for src/Makefile Alviro Iskandar Setiawan
  2022-03-08 22:40 ` [PATCH liburing 1/2] src/Makefile: Remove `-fomit-frame-pointer` from default build Alviro Iskandar Setiawan
@ 2022-03-08 22:40 ` Alviro Iskandar Setiawan
  2022-03-08 22:52   ` Ammar Faizi
  2022-03-08 22:58   ` Ammar Faizi
  1 sibling, 2 replies; 9+ messages in thread
From: Alviro Iskandar Setiawan @ 2022-03-08 22:40 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Alviro Iskandar Setiawan, Pavel Begunkov, Ammar Faizi,
	Alviro Iskandar Setiawan, io-uring, gwml

When the header files are modified, the compiled object are not going
to be recompiled because the header files are not marked as dependency
for the objects. Add those header files as dependency so it is safe
not to do "make clean" after changing those files.

Signed-off-by: Alviro Iskandar Setiawan <[email protected]>
---
 src/Makefile | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/src/Makefile b/src/Makefile
index f19d45e..b9428b7 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -46,9 +46,16 @@ endif
 liburing_objs := $(patsubst %.c,%.ol,$(liburing_srcs))
 liburing_sobjs := $(patsubst %.c,%.os,$(liburing_srcs))
 
-$(liburing_srcs): syscall.h lib.h
-
-$(liburing_objs) $(liburing_sobjs): include/liburing/io_uring.h
+$(liburing_objs) $(liburing_sobjs): \
+	syscall.h \
+	lib.h \
+	arch/syscall-defs.h \
+	arch/x86/syscall.h \
+	arch/x86/lib.h \
+	arch/aarch64/syscall.h \
+	arch/generic/syscall.h \
+	arch/generic/lib.h \
+	include/liburing/io_uring.h
 
 %.os: %.c
 	$(QUIET_CC)$(CC) $(CPPFLAGS) $(SO_CFLAGS) -c -o $@ $<
@@ -78,8 +85,6 @@ ifeq ($(ENABLE_SHARED),1)
 	ln -sf $(relativelibdir)$(libname) $(libdevdir)/liburing.so
 endif
 
-$(liburing_objs): include/liburing.h
-
 clean:
 	@rm -f $(all_targets) $(liburing_objs) $(liburing_sobjs) $(soname).new
 	@rm -f *.so* *.a *.o
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH liburing 2/2] src/Makefile: Add header files as dependency
  2022-03-08 22:40 ` [PATCH liburing 2/2] src/Makefile: Add header files as dependency Alviro Iskandar Setiawan
@ 2022-03-08 22:52   ` Ammar Faizi
  2022-03-08 23:06     ` Alviro Iskandar Setiawan
  2022-03-08 22:58   ` Ammar Faizi
  1 sibling, 1 reply; 9+ messages in thread
From: Ammar Faizi @ 2022-03-08 22:52 UTC (permalink / raw)
  To: Alviro Iskandar Setiawan, Jens Axboe
  Cc: Pavel Begunkov, Alviro Iskandar Setiawan, io-uring, gwml

On 3/9/22 5:40 AM, Alviro Iskandar Setiawan wrote:
> When the header files are modified, the compiled object are not going
> to be recompiled because the header files are not marked as dependency
> for the objects. Add those header files as dependency so it is safe
> not to do "make clean" after changing those files.
> 
> Signed-off-by: Alviro Iskandar Setiawan <[email protected]>
> ---
>   src/Makefile | 15 ++++++++++-----
>   1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/src/Makefile b/src/Makefile
> index f19d45e..b9428b7 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -46,9 +46,16 @@ endif
>   liburing_objs := $(patsubst %.c,%.ol,$(liburing_srcs))
>   liburing_sobjs := $(patsubst %.c,%.os,$(liburing_srcs))
>   
> -$(liburing_srcs): syscall.h lib.h
> -
> -$(liburing_objs) $(liburing_sobjs): include/liburing/io_uring.h
> +$(liburing_objs) $(liburing_sobjs): \
> +	syscall.h \
> +	lib.h \
> +	arch/syscall-defs.h \
> +	arch/x86/syscall.h \
> +	arch/x86/lib.h \
> +	arch/aarch64/syscall.h \
> +	arch/generic/syscall.h \
> +	arch/generic/lib.h \
> +	include/liburing/io_uring.h

This is ugly, it blindly adds all of them to the dependency while
they're actually not dependencies for all the C files here. For
example, when compiling for x86, we don't touch aarch64 files.

It is not a problem for liburing at the moment, because we don't
have many files in the src directory now. But I think we better
provide a long term solution on this.

For the headers files, I think we should rely on the compilers to
generate the dependency list with something like:

    "-MT ... -MMD -MP -MF"

Then include the generated dependency list to the Makefile.

What do you think?

-- 
Ammar Faizi


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH liburing 2/2] src/Makefile: Add header files as dependency
  2022-03-08 22:40 ` [PATCH liburing 2/2] src/Makefile: Add header files as dependency Alviro Iskandar Setiawan
  2022-03-08 22:52   ` Ammar Faizi
@ 2022-03-08 22:58   ` Ammar Faizi
  2022-03-08 23:07     ` Alviro Iskandar Setiawan
  1 sibling, 1 reply; 9+ messages in thread
From: Ammar Faizi @ 2022-03-08 22:58 UTC (permalink / raw)
  To: Alviro Iskandar Setiawan, Jens Axboe
  Cc: Pavel Begunkov, Alviro Iskandar Setiawan, io-uring, gwml

On 3/9/22 5:40 AM, Alviro Iskandar Setiawan wrote:
> When the header files are modified, the compiled object are not going
> to be recompiled because the header files are not marked as dependency
> for the objects. Add those header files as dependency so it is safe
> not to do "make clean" after changing those files.

Another missing part is the test files, they should also be recompiled
when changes to files in src/ are made. With this change, they are not.

The same also for examples/.

-- 
Ammar Faizi

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH liburing 2/2] src/Makefile: Add header files as dependency
  2022-03-08 22:52   ` Ammar Faizi
@ 2022-03-08 23:06     ` Alviro Iskandar Setiawan
  2022-03-09  0:23       ` Alviro Iskandar Setiawan
  0 siblings, 1 reply; 9+ messages in thread
From: Alviro Iskandar Setiawan @ 2022-03-08 23:06 UTC (permalink / raw)
  To: Ammar Faizi
  Cc: Jens Axboe, Pavel Begunkov, io-uring Mailing list,
	GNU/Weeb Mailing List

On Wed, Mar 9, 2022 at 5:52 AM Ammar Faizi wrote:
> This is ugly, it blindly adds all of them to the dependency while
> they're actually not dependencies for all the C files here. For
> example, when compiling for x86, we don't touch aarch64 files.
>
> It is not a problem for liburing at the moment, because we don't
> have many files in the src directory now. But I think we better
> provide a long term solution on this.
>
> For the headers files, I think we should rely on the compilers to
> generate the dependency list with something like:
>
>     "-MT ... -MMD -MP -MF"
>
> Then include the generated dependency list to the Makefile.
>
> What do you think?

Yes, I think it's better to do that. I'll fix this in v2.
thx

-- Viro

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH liburing 2/2] src/Makefile: Add header files as dependency
  2022-03-08 22:58   ` Ammar Faizi
@ 2022-03-08 23:07     ` Alviro Iskandar Setiawan
  0 siblings, 0 replies; 9+ messages in thread
From: Alviro Iskandar Setiawan @ 2022-03-08 23:07 UTC (permalink / raw)
  To: Ammar Faizi
  Cc: Jens Axboe, Pavel Begunkov, io-uring Mailing list,
	GNU/Weeb Mailing List

On Wed, Mar 9, 2022 at 5:59 AM Ammar Faizi wrote:
> On 3/9/22 5:40 AM, Alviro Iskandar Setiawan wrote:
> > When the header files are modified, the compiled object are not going
> > to be recompiled because the header files are not marked as dependency
> > for the objects. Add those header files as dependency so it is safe
> > not to do "make clean" after changing those files.
>
> Another missing part is the test files, they should also be recompiled
> when changes to files in src/ are made. With this change, they are not.
>
> The same also for examples/.

oc oc, that will be separate patches. Will be done in v2.

-- Viro

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH liburing 2/2] src/Makefile: Add header files as dependency
  2022-03-08 23:06     ` Alviro Iskandar Setiawan
@ 2022-03-09  0:23       ` Alviro Iskandar Setiawan
  2022-03-09  0:41         ` Ammar Faizi
  0 siblings, 1 reply; 9+ messages in thread
From: Alviro Iskandar Setiawan @ 2022-03-09  0:23 UTC (permalink / raw)
  To: Ammar Faizi
  Cc: Jens Axboe, Pavel Begunkov, io-uring Mailing list,
	GNU/Weeb Mailing List

On Wed, Mar 9, 2022 at 6:06 AM Alviro Iskandar Setiawan wrote:
> On Wed, Mar 9, 2022 at 5:52 AM Ammar Faizi wrote:
> > This is ugly, it blindly adds all of them to the dependency while
> > they're actually not dependencies for all the C files here. For
> > example, when compiling for x86, we don't touch aarch64 files.
> >
> > It is not a problem for liburing at the moment, because we don't
> > have many files in the src directory now. But I think we better
> > provide a long term solution on this.
> >
> > For the headers files, I think we should rely on the compilers to
> > generate the dependency list with something like:
> >
> >     "-MT ... -MMD -MP -MF"
> >
> > Then include the generated dependency list to the Makefile.
> >
> > What do you think?
>
> Yes, I think it's better to do that. I'll fix this in v2.
> thx

Sir, I am a bit confused with the include dependency files to the Makefile.

I use like this:

   -MT <object_filename> -MMD -MP -MF <dependency_file>

the dependency file is generated, but how to include them dynamically?
I think it shouldn't be included one by one.

So after this

   [...] -MT "setup.os" -MMD -MP -MF ".deps/setup.os.d" [...]
   [...] -MT "queue.os" -MMD -MP -MF ".deps/queue.os.d" [...]
   [...] -MT "register.os" -MMD -MP -MF ".deps/register.os.d" [...]
   [...] -MT "syscall.os" -MMD -MP -MF ".deps/syscall.os.d" [...]

files .deps/{setup,queue,registers,syscall}.os.d are generated, but I
have to include them to Makefile right? How to include them all at
once?

-- Viro

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH liburing 2/2] src/Makefile: Add header files as dependency
  2022-03-09  0:23       ` Alviro Iskandar Setiawan
@ 2022-03-09  0:41         ` Ammar Faizi
  0 siblings, 0 replies; 9+ messages in thread
From: Ammar Faizi @ 2022-03-09  0:41 UTC (permalink / raw)
  To: Alviro Iskandar Setiawan
  Cc: Jens Axboe, Pavel Begunkov, Alviro Iskandar Setiawan,
	io-uring Mailing list, GNU/Weeb Mailing List

On 3/9/22 7:23 AM, Alviro Iskandar Setiawan wrote:
> On Wed, Mar 9, 2022 at 6:06 AM Alviro Iskandar Setiawan wrote:
>> On Wed, Mar 9, 2022 at 5:52 AM Ammar Faizi wrote:
>>> This is ugly, it blindly adds all of them to the dependency while
>>> they're actually not dependencies for all the C files here. For
>>> example, when compiling for x86, we don't touch aarch64 files.
>>>
>>> It is not a problem for liburing at the moment, because we don't
>>> have many files in the src directory now. But I think we better
>>> provide a long term solution on this.
>>>
>>> For the headers files, I think we should rely on the compilers to
>>> generate the dependency list with something like:
>>>
>>>      "-MT ... -MMD -MP -MF"
>>>
>>> Then include the generated dependency list to the Makefile.
>>>
>>> What do you think?
>>
>> Yes, I think it's better to do that. I'll fix this in v2.
>> thx
> 
> Sir, I am a bit confused with the include dependency files to the Makefile.
> 
> I use like this:
> 
>     -MT <object_filename> -MMD -MP -MF <dependency_file>
> 
> the dependency file is generated, but how to include them dynamically?
> I think it shouldn't be included one by one.
> 
> So after this
> 
>     [...] -MT "setup.os" -MMD -MP -MF ".deps/setup.os.d" [...]
>     [...] -MT "queue.os" -MMD -MP -MF ".deps/queue.os.d" [...]
>     [...] -MT "register.os" -MMD -MP -MF ".deps/register.os.d" [...]
>     [...] -MT "syscall.os" -MMD -MP -MF ".deps/syscall.os.d" [...]
> 
> files .deps/{setup,queue,registers,syscall}.os.d are generated, but I
> have to include them to Makefile right? How to include them all at
> once?

Untested, but I think you can do something like:

   -include $(liburing_objs:%=.deps/%.d)

where liburing_objs is the variable that contains:
{setup,queue,registers,syscall}.os.

-- 
Ammar Faizi

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-03-09  1:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-03-08 22:40 [PATCH liburing 0/2] Changes for src/Makefile Alviro Iskandar Setiawan
2022-03-08 22:40 ` [PATCH liburing 1/2] src/Makefile: Remove `-fomit-frame-pointer` from default build Alviro Iskandar Setiawan
2022-03-08 22:40 ` [PATCH liburing 2/2] src/Makefile: Add header files as dependency Alviro Iskandar Setiawan
2022-03-08 22:52   ` Ammar Faizi
2022-03-08 23:06     ` Alviro Iskandar Setiawan
2022-03-09  0:23       ` Alviro Iskandar Setiawan
2022-03-09  0:41         ` Ammar Faizi
2022-03-08 22:58   ` Ammar Faizi
2022-03-08 23:07     ` Alviro Iskandar Setiawan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox