All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Leizhen (ThunderTown)" <thunder.leizhen@huawei.com>
To: David Laight <David.Laight@ACULAB.COM>,
	'Steven Rostedt' <rostedt@goodmis.org>
Cc: 'Andreas Schwab' <schwab@linux-m68k.org>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Josh Poimboeuf <jpoimboe@kernel.org>,
	Jiri Kosina <jikos@kernel.org>, Miroslav Benes <mbenes@suse.cz>,
	Petr Mladek <pmladek@suse.com>,
	Joe Lawrence <joe.lawrence@redhat.com>,
	"live-patching@vger.kernel.org" <live-patching@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Alexei Starovoitov <ast@kernel.org>, Jiri Olsa <jolsa@kernel.org>,
	Kees Cook <keescook@chromium.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Luis Chamberlain <mcgrof@kernel.org>,
	"linux-modules@vger.kernel.org" <linux-modules@vger.kernel.org>,
	Ingo Molnar <mingo@redhat.com>,
	linux-m68k <linux-m68k@lists.linux-m68k.org>,
	"Jason A. Donenfeld" <Jason@zx2c4.com>
Subject: Re: [PATCH v9] kallsyms: Add self-test facility
Date: Sat, 17 Dec 2022 15:31:39 +0800	[thread overview]
Message-ID: <066d4a61-3df9-b9c8-81a2-40dfcd3c73ef@huawei.com> (raw)
In-Reply-To: <83701d57c0064c709669c03cecc38356@AcuMS.aculab.com>



On 2022/12/17 3:27, David Laight wrote:
> From: Steven Rostedt
>> Sent: 16 December 2022 17:38
>>
>> On Fri, 16 Dec 2022 12:19:47 -0500
>> Steven Rostedt <rostedt@goodmis.org> wrote:
>>
>>> I assumed that "memory" was for memory unrelated to the input constraints.
>>
>> Well, it looks like you do need a "memory" barrier.
>>
>>   https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html
>>
>> "memory"
>>
>>       The "memory" clobber tells the compiler that the assembly code
>>       performs memory reads or writes to items other than those listed in
>>       the input and output operands (for example, accessing the memory
>>       pointed to by one of the input parameters). To ensure memory contains
>>       correct values, GCC may need to flush specific register values to
>>       memory before executing the asm. Further, the compiler does not
>>       assume that any values read from memory before an asm remain
>>       unchanged after that asm; it reloads them as needed. Using the
>>       "memory" clobber effectively forms a read/write memory barrier for
>>       the compiler.
>>
>> As the "(for example, accessing the memory pointed to by one of the input
>> parameters)" is exactly this case.
> 
> Without the memory clobber code like:
> int f(const char *s)
> {
> 	char c[4] = "abc";
> 	return strcmp(s, c);
> }
> is very like to get optimised so that c[] is never written.
> 
> However, in this case, the strings have all existed for ages.
> So that won't be the problem.
> 
> It might be obvious what is wrong from the asm output.
> Although the binary-chop lookup is suspect I'd also check
> that the sorted index is plausible - just tracing the first
> 20 entries might be enough.
> No point peering at the search code if the setup is wrong.

6.47.2.1 Volatile
GCC’s optimizers sometimes discard asm statements if they determine there is no need for
the output variables. Also, the optimizers may move code out of loops if they believe that
the code will always return the same result (i.e. none of its input values change between
calls). Using the volatile qualifier disables these optimizations.

So it's quite possible (I didn't disassemble vmlinux, because I didn't learn m68k):

//binary search
while (low <= high) {
    ...
    ret = compare_symbol_name(name, namebuf);   ----> (1)
    if (!ret)
        break;
}

low = mid;
while (low) {
    ...
    if (compare_symbol_name(name, namebuf))     ----> (2)
        break;
    low--;
}

The pointer 'name' and 'namebuf' of (1) and (2) are the same,
so the 'if' statement of (2) maybe omitted by compiler.

By the way, I tried no volatile but with
+               : : "memory");
It also works well.

> 
> 	David
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
> 
> 
> .
> 

-- 
Regards,
  Zhen Lei

  reply	other threads:[~2022-12-17  7:32 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-15  8:33 [PATCH v9] kallsyms: Add self-test facility Zhen Lei
2022-11-15  8:42 ` Luis Chamberlain
2022-11-15  8:43 ` Leizhen (ThunderTown)
2022-11-22 12:29 ` kernel test robot
2022-11-22 13:24   ` Leizhen (ThunderTown)
2022-11-22 15:31 ` kernel test robot
2022-12-15  8:50 ` Geert Uytterhoeven
2022-12-15  9:16   ` Leizhen (ThunderTown)
2022-12-15  9:39     ` Geert Uytterhoeven
2022-12-15 12:33       ` Leizhen (ThunderTown)
2022-12-15 13:24         ` Geert Uytterhoeven
2022-12-15 13:58           ` Leizhen (ThunderTown)
2022-12-15 14:40             ` Leizhen (ThunderTown)
2022-12-15 14:51               ` Geert Uytterhoeven
2022-12-16  7:42                 ` Leizhen (ThunderTown)
2022-12-16  9:36                   ` Leizhen (ThunderTown)
2022-12-16 11:28                     ` Geert Uytterhoeven
2022-12-16 12:01                       ` Leizhen (ThunderTown)
2022-12-16 13:29                         ` David Laight
2022-12-16 14:44                           ` David Laight
2022-12-16 15:19                             ` Andreas Schwab
2022-12-16 15:25                               ` David Laight
2022-12-16 15:54                                 ` Andreas Schwab
2022-12-16 16:09                                   ` David Laight
2022-12-16 16:11                                     ` Andreas Schwab
2022-12-16 16:32                                       ` David Laight
2022-12-16 16:53                                         ` Steven Rostedt
2022-12-16 16:57                                           ` David Laight
2022-12-16 17:19                                             ` Steven Rostedt
2022-12-16 17:38                                               ` Steven Rostedt
2022-12-16 19:27                                                 ` David Laight
2022-12-17  7:31                                                   ` Leizhen (ThunderTown) [this message]
2022-12-17 13:37                                                     ` Geert Uytterhoeven
2022-12-17 17:37                                                       ` David Laight
2022-12-16 11:57                     ` Andreas Schwab
2022-12-16 13:31                       ` Leizhen (ThunderTown)
2022-12-16 13:47                         ` Andreas Schwab
2022-12-15 14:43             ` Geert Uytterhoeven
2022-12-15 14:51               ` Andreas Schwab
2022-12-16 10:40           ` David Laight
2022-12-16 11:40             ` Leizhen (ThunderTown)
2022-12-16 11:44           ` Andreas Schwab
2022-12-16 11:57             ` Leizhen (ThunderTown)
2022-12-16 12:39               ` Andreas Schwab
2022-12-16 13:31                 ` Leizhen (ThunderTown)

Reply instructions:

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=066d4a61-3df9-b9c8-81a2-40dfcd3c73ef@huawei.com \
    --to=thunder.leizhen@huawei.com \
    --cc=David.Laight@ACULAB.COM \
    --cc=Jason@zx2c4.com \
    --cc=akpm@linux-foundation.org \
    --cc=ast@kernel.org \
    --cc=geert@linux-m68k.org \
    --cc=jikos@kernel.org \
    --cc=joe.lawrence@redhat.com \
    --cc=jolsa@kernel.org \
    --cc=jpoimboe@kernel.org \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-m68k@lists.linux-m68k.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=masahiroy@kernel.org \
    --cc=mbenes@suse.cz \
    --cc=mcgrof@kernel.org \
    --cc=mingo@redhat.com \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=schwab@linux-m68k.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.