Re: [linux-safety] [PATCH] mm/hugetlb.c: remove deadstore in demote_size_show()

--000000000000dc991805cd5d5e93
Content-Type: text/plain; charset="UTF-8"

Dear Lukas,
Thank you for detailed comments as well as your help. Patch v2 is ready for
next review.
Best regards,
Nghia Le

On Sat, Oct 2, 2021 at 12:14 PM Lukas Bulwahn <[email protected]>
wrote:

> Just some minor improvements for the language:
>
> On Fri, Oct 1, 2021 at 7:57 PM Nghia Le <[email protected]> wrote:
> >
> > Deadstore was detected by CodeCheck tool (ELISA group)
>
> Hints:
>   - Use active instead of passive tense
>   - It is detected by "make clang-analyzer". The CodeChecker tool is
> just the webUI frontend.
>   - Dead store is written as two words (with a space between dead and
> store).
>
> So, make it:
> The command "make clang-analyzer" detected a dead store.
>
> >
> > Removed demote_size and relevant assignment in function
> demote_size_show()
> > to fix deadstore issue of which stored value to demote_size is never read
> >
>
> Hints:
>   - Use imperative form.
>   - ... and just some better English grammar.
>
> So make it:
> Remove demote_size and corresponding assignment in function
> demote_size_show()
> to fix dead store, as demote_size is never read.
>
> Also change deadstore to dead store in subject.
>
> Other than that, it looks good to me.
>
> Send a PATCH v2 here. Then, we check again and you can send the patch
> out to the maintainers.
>
> Lukas
>
> > Signed-off-by: Nghia Le <[email protected]>
> > ---
> >  mm/hugetlb.c | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > index 993efa70bce4..ef00e6ad0f6a 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -3706,11 +3706,9 @@ static ssize_t demote_size_show(struct kobject
> *kobj,
> >                                         struct kobj_attribute *attr,
> char *buf)
> >  {
> >         struct hstate *h;
> > -       unsigned long demote_size;
> >         int nid;
> >
> >         h = kobj_to_hstate(kobj, &nid);
> > -       demote_size = h->demote_order;
> >
> >         return sysfs_emit(buf, "%lukB\n",
> >                         (unsigned long)(PAGE_SIZE << h->demote_order) /
> SZ_1K);
> > --
> > 2.25.1
> >
>


-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#250): https://lists.elisa.tech/g/linux-safety/message/250
Mute This Topic: https://lists.elisa.tech/mt/86006510/5278000
Group Owner: [email protected]
Unsubscribe: https://lists.elisa.tech/g/linux-safety/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-



--000000000000dc991805cd5d5e93
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">Dear Lukas,<div>Thank you for detailed comments as well as=
 your help. Patch v2 is ready for next review.</div><div>Best regards,</div=
><div>Nghia Le</div></div><br><div class=3D"gmail_quote"><div dir=3D"ltr" c=
lass=3D"gmail_attr">On Sat, Oct 2, 2021 at 12:14 PM Lukas Bulwahn &lt;<a hr=
ef=3D"mailto:[email protected]">[email protected]</a>&gt; wrote=
:<br></div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.=
8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Just some mino=
r improvements for the language:<br>
<br>
On Fri, Oct 1, 2021 at 7:57 PM Nghia Le &lt;<a href=3D"mailto:nghialm78@gma=
il.com" target=3D"_blank">[email protected]</a>&gt; wrote:<br>
&gt;<br>
&gt; Deadstore was detected by CodeCheck tool (ELISA group)<br>
<br>
Hints:<br>
=C2=A0 - Use active instead of passive tense<br>
=C2=A0 - It is detected by &quot;make clang-analyzer&quot;. The CodeChecker=
 tool is<br>
just the webUI frontend.<br>
=C2=A0 - Dead store is written as two words (with a space between dead and =
store).<br>
<br>
So, make it:<br>
The command &quot;make clang-analyzer&quot; detected a dead store.<br>
<br>
&gt;<br>
&gt; Removed demote_size and relevant assignment in function demote_size_sh=
ow()<br>
&gt; to fix deadstore issue of which stored value to demote_size is never r=
ead<br>
&gt;<br>
<br>
Hints:<br>
=C2=A0 - Use imperative form.<br>
=C2=A0 - ... and just some better English grammar.<br>
<br>
So make it:<br>
Remove demote_size and corresponding assignment in function demote_size_sho=
w()<br>
to fix dead store, as demote_size is never read.<br>
<br>
Also change deadstore to dead store in subject.<br>
<br>
Other than that, it looks good to me.<br>
<br>
Send a PATCH v2 here. Then, we check again and you can send the patch<br>
out to the maintainers.<br>
<br>
Lukas<br>
<br>
&gt; Signed-off-by: Nghia Le &lt;<a href=3D"mailto:[email protected]" tar=
get=3D"_blank">[email protected]</a>&gt;<br>
&gt; ---<br>
&gt;=C2=A0 mm/hugetlb.c | 2 --<br>
&gt;=C2=A0 1 file changed, 2 deletions(-)<br>
&gt;<br>
&gt; diff --git a/mm/hugetlb.c b/mm/hugetlb.c<br>
&gt; index 993efa70bce4..ef00e6ad0f6a 100644<br>
&gt; --- a/mm/hugetlb.c<br>
&gt; +++ b/mm/hugetlb.c<br>
&gt; @@ -3706,11 +3706,9 @@ static ssize_t demote_size_show(struct kobject =
*kobj,<br>
&gt;=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0struct kobj_attribute *attr, char *buf)<br>
&gt;=C2=A0 {<br>
&gt;=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0struct hstate *h;<br>
&gt; -=C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned long demote_size;<br>
&gt;=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0int nid;<br>
&gt;<br>
&gt;=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0h =3D kobj_to_hstate(kobj, &amp;nid);=
<br>
&gt; -=C2=A0 =C2=A0 =C2=A0 =C2=A0demote_size =3D h-&gt;demote_order;<br>
&gt;<br>
&gt;=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return sysfs_emit(buf, &quot;%lukB\n&=
quot;,<br>
&gt;=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0(unsigned long)(PAGE_SIZE &lt;&lt; h-&gt;demote_order) =
/ SZ_1K);<br>
&gt; --<br>
&gt; 2.25.1<br>
&gt;<br>
</blockquote></div>


 <div width=3D"1" style=3D"color:white;clear:both">_._,_._,_</div> <hr>   L=
inks:<p>   You receive all messages sent to this group.    <p> <a target=3D=
"_blank" href=3D"https://lists.elisa.tech/g/linux-safety/message/250">View/=
Reply Online (#250)</a> |  <a target=3D"_blank" href=3D"mailto:nghialm78@gm=
ail.com?subject=3DPrivate:%20Re:%20Re%3A%20%5Blinux-safety%5D%20%5BPATCH%5D=
%20mm%2Fhugetlb.c%3A%20remove%20deadstore%20in%20demote_size_show%28%29">Re=
ply To Sender</a>  | <a target=3D"_blank" href=3D"mailto:linux-safety@lists=
.elisa.tech?subject=3DRe:%20Re%3A%20%5Blinux-safety%5D%20%5BPATCH%5D%20mm%2=
Fhugetlb.c%3A%20remove%20deadstore%20in%20demote_size_show%28%29">Reply To =
Group</a>   |  <a target=3D"_blank" href=3D"https://lists.elisa.tech/mt/860=
06510/5278000">Mute This Topic</a>  | <a href=3D"https://lists.elisa.tech/g=
/linux-safety/post">New Topic</a><br>    <a href=3D"https://lists.elisa.tec=
h/g/linux-safety/editsub/5278000">Your Subscription</a> | <a href=3D"mailto=
:[email protected]">Contact Group Owner</a> |  <a href=3D=
"https://lists.elisa.tech/g/linux-safety/unsub">Unsubscribe</a>  [linux-saf=
[email protected]]<br> <div width=3D"1" style=3D"color:white;clear:bo=
th">_._,_._,_</div>=20


--000000000000dc991805cd5d5e93--