From: "Wang, Haiyue" <haiyue.wang@linux.intel.com>
To: minyard@acm.org, openipmi-developer@lists.sourceforge.net,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH ipmi/kcs_bmc v1] ipmi: kcs_bmc: optimize the data buffers allocation
Date: Sat, 7 Apr 2018 10:37:22 +0800 [thread overview]
Message-ID: <dd4e07c4-7a99-3449-fd2d-58c2b829f7b7@linux.intel.com> (raw)
In-Reply-To: <dd59cf14-e269-6c00-9e33-87316cdc8e8f@gmail.com>
On 2018-04-07 05:47, Corey Minyard wrote:
> On 03/15/2018 07:20 AM, Haiyue Wang wrote:
>> Allocate a continuous memory block for the three KCS data buffers with
>> related index assignment.
>
> I'm finally getting to this.
>
> Is there a reason you want to do this? In general, it's better to not
> try to
> outsmart your base system. Depending on the memory allocator, in this
> case, you might actually use more memory. You probably won't use any
> less.
>
I got this idea from another code review, but that patch allocates 30 more
the same size memory block, reducing the devm_kmalloc call will be better.
For KCS only have 3, may be the key point is memory waste.
> In the original case, you allocate three 1000 byte buffers, resulting
> in 3
> 1024 byte slab allocated.
>
> In the changed case, you will allocate a 3000 byte buffer, resulting in
> a single 4096 byte slab allocation, wasting 1024 more bytes of memory.
>
As the kcs has memory copy between in/out/kbuffer, put them in the same
page will be better ? Such as the same TLB ? (Well, I just got this from
book,
no real experience of memory accessing performance. And also, I was told
that using space to save the time. :-)).
Just my stupid thinking. I'm OK to drop this patch if it doesn't help with
performance, or something else.
BR.
Haiyue
> -corey
>
>> Signed-off-by: Haiyue Wang <haiyue.wang@linux.intel.com>
>> ---
>> drivers/char/ipmi/kcs_bmc.c | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/char/ipmi/kcs_bmc.c b/drivers/char/ipmi/kcs_bmc.c
>> index fbfc05e..dc19c0d 100644
>> --- a/drivers/char/ipmi/kcs_bmc.c
>> +++ b/drivers/char/ipmi/kcs_bmc.c
>> @@ -435,6 +435,7 @@ static const struct file_operations kcs_bmc_fops = {
>> struct kcs_bmc *kcs_bmc_alloc(struct device *dev, int sizeof_priv,
>> u32 channel)
>> {
>> struct kcs_bmc *kcs_bmc;
>> + void *buf;
>> kcs_bmc = devm_kzalloc(dev, sizeof(*kcs_bmc) + sizeof_priv,
>> GFP_KERNEL);
>> if (!kcs_bmc)
>> @@ -448,11 +449,12 @@ struct kcs_bmc *kcs_bmc_alloc(struct device
>> *dev, int sizeof_priv, u32 channel)
>> mutex_init(&kcs_bmc->mutex);
>> init_waitqueue_head(&kcs_bmc->queue);
>> - kcs_bmc->data_in = devm_kmalloc(dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
>> - kcs_bmc->data_out = devm_kmalloc(dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
>> - kcs_bmc->kbuffer = devm_kmalloc(dev, KCS_MSG_BUFSIZ, GFP_KERNEL);
>> - if (!kcs_bmc->data_in || !kcs_bmc->data_out || !kcs_bmc->kbuffer)
>> + buf = devm_kmalloc_array(dev, 3, KCS_MSG_BUFSIZ, GFP_KERNEL);
>> + if (!buf)
>> return NULL;
>> + kcs_bmc->data_in = buf;
>> + kcs_bmc->data_out = buf + KCS_MSG_BUFSIZ;
>> + kcs_bmc->kbuffer = buf + KCS_MSG_BUFSIZ * 2;
>> kcs_bmc->miscdev.minor = MISC_DYNAMIC_MINOR;
>> kcs_bmc->miscdev.name = dev_name(dev);
>
>
next prev parent reply other threads:[~2018-04-07 2:37 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-15 12:20 [PATCH ipmi/kcs_bmc v1] ipmi: kcs_bmc: optimize the data buffers allocation Haiyue Wang
2018-04-03 6:00 ` Wang, Haiyue
2018-04-03 18:45 ` Corey Minyard
2018-04-04 0:34 ` Wang, Haiyue
2018-04-06 21:47 ` Corey Minyard
2018-04-07 2:37 ` Wang, Haiyue [this message]
2018-04-07 7:54 ` Wang, Haiyue
2018-04-13 13:50 ` Corey Minyard
2018-04-13 14:10 ` Wang, Haiyue
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=dd4e07c4-7a99-3449-fd2d-58c2b829f7b7@linux.intel.com \
--to=haiyue.wang@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=minyard@acm.org \
--cc=openipmi-developer@lists.sourceforge.net \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).