| Message ID | 7f072f78-eef4-6d87-d233-cee71dac5a32@users.sourceforge.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello,
I came along some software modules where I suggested source code adjustments.
Example:
nfs/write: Use common error handling code in nfs_lock_and_join_requests()
https://lkml.org/lkml/2017/11/7/599
https://patchwork.kernel.org/patch/10047013/
https://lkml.kernel.org/r/<7f072f78-eef4-6d87-d233-cee71dac5a32@users.sourceforge.net>
I would like to check corresponding build results then without extra
optimisation applied by the compiler.
But I got surprised by error messages for a command like the following.
elfring@Sonne:~/Projekte/Linux/next-patched> my_cc=/usr/bin/gcc-7 && LANG=C make -j4 CC="${my_cc}" HOSTCC="${my_cc}" EXTRA_CFLAGS='-O0' allmodconfig fs/nfs/write.o
…
In file included from ./include/linux/compiler.h:58:0,
from ./include/uapi/linux/stddef.h:1,
from ./include/linux/stddef.h:4,
from ./include/uapi/linux/posix_types.h:4,
from ./include/uapi/linux/types.h:13,
from ./include/linux/types.h:5,
from fs/nfs/write.c:9:
./arch/x86/include/asm/jump_label.h: In function ‘trace_nfs_writeback_page_enter’:
./include/linux/compiler-gcc.h:275:38: warning: asm operand 0 probably doesn’t match constraints
#define asm_volatile_goto(x...) do { asm goto(x); asm (""); } while (0)
…
How do you think about to improve this software situation anyhow?
Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 2017-12-03 at 15:15 +0100, SF Markus Elfring wrote: > Hello, > > I came along some software modules where I suggested source code > adjustments. > > Example: > nfs/write: Use common error handling code in > nfs_lock_and_join_requests() > > https://lkml.org/lkml/2017/11/7/599 > https://patchwork.kernel.org/patch/10047013/ > https://lkml.kernel.org/r/<7f072f78-eef4-6d87-d233-cee71dac5a32@users > .sourceforge.net>; > > I would like to check corresponding build results then without extra > optimisation applied by the compiler. > But I got surprised by error messages for a command like the > following. > > elfring@Sonne:~/Projekte/Linux/next-patched> my_cc=/usr/bin/gcc-7 && > LANG=C make -j4 CC="${my_cc}" HOSTCC="${my_cc}" EXTRA_CFLAGS='-O0' > allmodconfig fs/nfs/write.o > … > In file included from ./include/linux/compiler.h:58:0, > from ./include/uapi/linux/stddef.h:1, > from ./include/linux/stddef.h:4, > from ./include/uapi/linux/posix_types.h:4, > from ./include/uapi/linux/types.h:13, > from ./include/linux/types.h:5, > from fs/nfs/write.c:9: > ./arch/x86/include/asm/jump_label.h: In function > ‘trace_nfs_writeback_page_enter’: > ./include/linux/compiler-gcc.h:275:38: warning: asm operand 0 > probably doesn’t match constraints > #define asm_volatile_goto(x...) do { asm goto(x); asm (""); } while > (0) > … > > > How do you think about to improve this software situation anyhow? I'm not seeing anything obviously wrong with the NFS use of tracepoints there, and the warning suggests rather that gcc has an issue with the inlined assembly code in jump_label.h. Ccing Peter Zijlstra (who appears to have been the last person to touch that assembly code) and Steven Rostedt. -- Trond Myklebust Linux NFS client maintainer, PrimaryData trond.myklebust@primarydata.com
On Sun, 3 Dec 2017 15:17:32 +0000 Trond Myklebust <trondmy@primarydata.com> wrote: > > I would like to check corresponding build results then without extra > > optimisation applied by the compiler. > > But I got surprised by error messages for a command like the > > following. > > > > elfring@Sonne:~/Projekte/Linux/next-patched> my_cc=/usr/bin/gcc-7 && > > LANG=C make -j4 CC="${my_cc}" HOSTCC="${my_cc}" EXTRA_CFLAGS='-O0' > > allmodconfig fs/nfs/write.o Why would you compile the kernel without optimization? There's many places in the kernel that WILL NOT BUILD without optimization. In fact, we do a lot of tricks to make sure that things work the way we expect it to, because we add broken code that only gets compiled out when gcc optimizes the code the way we expect it to be, and the kernel build will break otherwise. -- Steve -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Why would you compile the kernel without optimization? I would like to see how big an effect finally is in such a build configuration after specific source code adjustments. > There's many places in the kernel that WILL NOT BUILD without optimization. I did not really know this detail so far. I noticed that the optimised build variants worked during my test comparisons. > In fact, we do a lot of tricks to make sure that things work the way > we expect it to, because we add broken code that only gets compiled out > when gcc optimizes the code the way we expect it to be, > and the kernel build will break otherwise. Thanks for your information. Can the software areas distinguished where such special handling matters? Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 3 Dec 2017 22:56:51 +0100
SF Markus Elfring <elfring@users.sourceforge.net> wrote:
> Can the software areas distinguished where such special handling matters?
No idea. That's something you are going to have to figure out on your
own.
-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
> Why would you compile the kernel without optimization? Can another reason be occasionally still relevant? Will the compilation be a bit quicker when extra data processing could be omitted? > There's many places in the kernel that WILL NOT BUILD without optimization. Would you like to keep the software situation in this way? > In fact, we do a lot of tricks to make sure that things work the way > we expect it to, because we add broken code that only gets compiled out > when gcc optimizes the code the way we expect it to be, > and the kernel build will break otherwise. * Can this goal be also achieved without the addition of “broken code”? * How do you think about to improve the error handling there? Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 4 Dec 2017 10:00:54 +0100 SF Markus Elfring <elfring@users.sourceforge.net> wrote: > > Why would you compile the kernel without optimization? > > Can another reason be occasionally still relevant? No. > > Will the compilation be a bit quicker when extra data processing > could be omitted? Why would you care more about the time it takes to compile the kernel, than the time it takes for executing it? Benchmarks are all about performance of a running kernel, nobody compares benchmarks of the time it takes to compile it. Sure, we like to make the compile times quicker (heck, I wrote "make localmodconfig" for just that purpose), but we never favor compiler time over execution time. In fact, if we can improve the execution performance by sacrificing compile time, we are happy to do that. > > > > There's many places in the kernel that WILL NOT BUILD without optimization. > > Would you like to keep the software situation in this way? Yes. > > > > In fact, we do a lot of tricks to make sure that things work the way > > we expect it to, because we add broken code that only gets compiled out > > when gcc optimizes the code the way we expect it to be, > > and the kernel build will break otherwise. > > * Can this goal be also achieved without the addition of “broken code”? No. > > * How do you think about to improve the error handling there? It works just fine as is. Errors that can be detected at build time are 100 times better than detecting them at execution time. -- Steve -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Can the software areas distinguished where such special handling matters? > > No idea. I would like to point another example out. > That's something you are going to have to figure out on your own. How do you think about information from an other clarification request for the topic “caif: Use common error handling code in cfdgml_receive()”? https://lkml.org/lkml/2017/11/7/486 https://patchwork.kernel.org/patch/10046849/ https://lkml.kernel.org/r/<034c0760-b8af-0fe2-e7a0-81199ff31931@users.sourceforge.net> Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Will the compilation be a bit quicker when extra data processing >> could be omitted? > > Why would you care more about the time it takes to compile the kernel, > than the time it takes for executing it? I am also interested in the evolution of compilation time frames. > Benchmarks are all about performance of a running kernel, This is generally reasonable. > nobody compares benchmarks of the time it takes to compile it. I guess that the situation can be occasionally different there. > Sure, we like to make the compile times quicker Good to know … > (heck, I wrote "make localmodconfig" for just that purpose), Thanks. > but we never favor compiler time over execution time. I imagine that the speed expectations could be adjusted during software development, couldn't they? > In fact, if we can improve the execution performance by sacrificing compile time, > we are happy to do that. I guess that you would like to consider some constraints there. >>> In fact, we do a lot of tricks to make sure that things work the way >>> we expect it to, because we add broken code that only gets compiled out >>> when gcc optimizes the code the way we expect it to be, >>> and the kernel build will break otherwise. >> >> * Can this goal be also achieved without the addition of “broken code”? > > No. Will any other contributors take another look? >> * How do you think about to improve the error handling there? > > It works just fine as is. I hope that further software improvements can be achieved also for this use case. > Errors that can be detected at build time are 100 times better > than detecting them at execution time. I agree to such a general view. Will an other (or no) error message be more appropriate? Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/nfs/write.c b/fs/nfs/write.c index babebbccae2a..5b5f464f6f2a 100644 --- a/fs/nfs/write.c +++ b/fs/nfs/write.c @@ -487,10 +487,8 @@ nfs_lock_and_join_requests(struct page *page) } ret = nfs_page_group_lock(head); - if (ret < 0) { - nfs_unlock_and_release_request(head); - return ERR_PTR(ret); - } + if (ret < 0) + goto release_request; /* lock each request in the page group */ total_bytes = head->wb_bytes; @@ -515,8 +513,7 @@ nfs_lock_and_join_requests(struct page *page) if (ret < 0) { nfs_unroll_locks(inode, head, subreq); nfs_release_request(subreq); - nfs_unlock_and_release_request(head); - return ERR_PTR(ret); + goto release_request; } } /* @@ -532,8 +529,8 @@ nfs_lock_and_join_requests(struct page *page) nfs_page_group_unlock(head); nfs_unroll_locks(inode, head, subreq); nfs_unlock_and_release_request(subreq); - nfs_unlock_and_release_request(head); - return ERR_PTR(-EIO); + ret = -EIO; + goto release_request; } } @@ -576,6 +573,10 @@ nfs_lock_and_join_requests(struct page *page) /* still holds ref on head from nfs_page_find_head_request * and still has lock on head from lock loop */ return head; + +release_request: + nfs_unlock_and_release_request(head); + return ERR_PTR(ret); } static void nfs_write_error_remove_page(struct nfs_page *req)