| Message ID | 20211003114113.109463-1-nghialm78@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/hugetlb.c: remove dead store in demote_size_show() | expand |
On Sun, Oct 03, 2021 at 06:41:13PM +0700, Nghia Le wrote: > { > 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); I'd suggest this function would look better written as: int nid; struct hstate *h = kobj_to_hstate(kobj, &nid); unsigned long demote_size = (PAGE_SIZE << h->demote_order) / SZ_1K; return sysfs_emit(buf, "%lukB\n", demote_size);
On 10/3/21 6:54 AM, Matthew Wilcox wrote: > On Sun, Oct 03, 2021 at 06:41:13PM +0700, Nghia Le wrote: >> { >> 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); > > I'd suggest this function would look better written as: > > int nid; > struct hstate *h = kobj_to_hstate(kobj, &nid); > unsigned long demote_size = (PAGE_SIZE << h->demote_order) / SZ_1K; > > return sysfs_emit(buf, "%lukB\n", demote_size); > Thank you Nghia Le for spotting this, and thank you Matthew for the suggestion. This is still just in Andrew's tree and subject to modification before the next merge window. I am still expecting additional comments on the series. If another version of the series is needed, I will include Matthew's suggestion. If not, I will ask Andrew how he would prefer to fold in the changes.
On Sun, Oct 03, 2021 at 07:36:54PM -0700, Mike Kravetz wrote: > On 10/3/21 6:54 AM, Matthew Wilcox wrote: > > On Sun, Oct 03, 2021 at 06:41:13PM +0700, Nghia Le wrote: > >> { > >> 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); > > > > I'd suggest this function would look better written as: > > > > int nid; > > struct hstate *h = kobj_to_hstate(kobj, &nid); > > unsigned long demote_size = (PAGE_SIZE << h->demote_order) / SZ_1K; > > > > return sysfs_emit(buf, "%lukB\n", demote_size); > > Thanks Matthew for the clean code. > > Thank you Nghia Le for spotting this, and thank you Matthew for the > suggestion. > > This is still just in Andrew's tree and subject to modification before > the next merge window. I am still expecting additional comments on the > series. > > If another version of the series is needed, I will include Matthew's > suggestion. If not, I will ask Andrew how he would prefer to fold in > the changes. > -- > Mike Kravetz Thanks Mike, so we will wait further comments from Andrew and others.
On 10/3/21 8:44 PM, Nghia Le wrote: > On Sun, Oct 03, 2021 at 07:36:54PM -0700, Mike Kravetz wrote: >> On 10/3/21 6:54 AM, Matthew Wilcox wrote: >>> On Sun, Oct 03, 2021 at 06:41:13PM +0700, Nghia Le wrote: >>>> { >>>> 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); >>> >>> I'd suggest this function would look better written as: >>> >>> int nid; >>> struct hstate *h = kobj_to_hstate(kobj, &nid); >>> unsigned long demote_size = (PAGE_SIZE << h->demote_order) / SZ_1K; >>> >>> return sysfs_emit(buf, "%lukB\n", demote_size); >>> > Thanks Matthew for the clean code. >> >> Thank you Nghia Le for spotting this, and thank you Matthew for the >> suggestion. >> >> This is still just in Andrew's tree and subject to modification before >> the next merge window. I am still expecting additional comments on the >> series. >> >> If another version of the series is needed, I will include Matthew's >> suggestion. If not, I will ask Andrew how he would prefer to fold in >> the changes. >> -- >> Mike Kravetz > Thanks Mike, so we will wait further comments from Andrew and others. Yes, I received suggestions for other improvements to the patch which contains this code. I will be putting together another version of the series with at least these changes. Thanks,
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);
The command "make clang-analyzer" detected a dead store. Remove demote_size and corresponding assignment in function demote_size_show() to fix dead store, as demote_size is never read. Signed-off-by: Nghia Le <nghialm78@gmail.com> --- mm/hugetlb.c | 2 -- 1 file changed, 2 deletions(-)