All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@kernel.dk>
To: Adam Manzanares <Adam.Manzanares@wdc.com>,
	Hannes Reinecke <hare@suse.de>
Cc: "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	Mel Gorman <mgorman@suse.com>, Hannes Reinecke <hare@suse.com>
Subject: Re: [PATCH] brd: Allow ramdisk to be allocated on selected NUMA node
Date: Thu, 14 Jun 2018 14:47:39 -0600	[thread overview]
Message-ID: <5f21f241-0ae0-ed7a-6935-3ef6e65d0950@kernel.dk> (raw)
In-Reply-To: <53893c29-febc-14ff-314d-818ac79aa559@wdc.com>

On 6/14/18 2:41 PM, Adam Manzanares wrote:
> 
> 
> On 6/14/18 1:37 PM, Jens Axboe wrote:
>> On 6/14/18 2:32 PM, Adam Manzanares wrote:
>>>
>>>
>>> On 6/14/18 9:09 AM, Hannes Reinecke wrote:
>>>> On Thu, 14 Jun 2018 09:33:35 -0600
>>>> Jens Axboe <axboe@kernel.dk> wrote:
>>>>
>>>>> On 6/14/18 9:29 AM, Hannes Reinecke wrote:
>>>>>> On Thu, 14 Jun 2018 08:47:33 -0600
>>>>>> Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>     
>>>>>>> On 6/14/18 7:38 AM, Hannes Reinecke wrote:
>>>>>>>> For performance reasons we should be able to allocate all memory
>>>>>>>> from a given NUMA node, so this patch adds a new parameter
>>>>>>>> 'rd_numa_node' to allow the user to specify the NUMA node id.
>>>>>>>> When restricing fio to use the same NUMA node I'm seeing a
>>>>>>>> performance boost of more than 200%.
>>>>>>>
>>>>>>> Looks fine to me. One comment.
>>>>>>>    
>>>>>>>> @@ -342,6 +343,10 @@ static int max_part = 1;
>>>>>>>>    module_param(max_part, int, 0444);
>>>>>>>>    MODULE_PARM_DESC(max_part, "Num Minors to reserve between
>>>>>>>> devices");
>>>>>>>> +static int rd_numa_node = NUMA_NO_NODE;
>>>>>>>> +module_param(rd_numa_node, int, 0444);
>>>>>>>> +MODULE_PARM_DESC(rd_numa_node, "NUMA node number to allocate RAM
>>>>>>>> disk on.");
>>>>>>>
>>>>>>> This could feasibly be 0644, as there would be nothing wrong with
>>>>>>> altering this at runtime.
>>>>>>>    
>>>>>>
>>>>>> While we could it would not change the allocation of _existing_ ram
>>>>>> devices, making behaviour rather unpredictable.
>>>>>> Hence I did decide against it (and yes, I actually thought about
>>>>>> it).
>>>>>>
>>>>>> But if you insist ...
>>>>>
>>>>> Right, it would just change new allocations. Probably not a common use
>>>>> case, but there's really nothing that prevents it from being feasible.
>>>>>
>>>>> Next question - what does the memory allocator do if we run out of
>>>>> memory on the given node? Should we punt to a different node if that
>>>>> happens? Slower, but functional, seems preferable to not being able
>>>>> to get memory.
>>>>>
>>>>
>>>> Hmm. That I haven't considered; yes, that really sounds like an idea.
>>>> Will be sending an updated patch.
>>>
>>> Will numactl ... modprobe brd ... solve this problem?
>>
>> It won't, pages are allocated as needed.
>>
> 
> Then how about a numactl ... dd /dev/ram ... after the modprobe.

Yes of course, or you could do that for every application that ends
up in the path of the doing IO to it. The point of the option is to
just make it explicit, and not have to either NUMA pin each task,
or prefill all possible pages.

-- 
Jens Axboe

  reply	other threads:[~2018-06-14 20:47 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-14 13:38 [PATCH] brd: Allow ramdisk to be allocated on selected NUMA node Hannes Reinecke
2018-06-14 14:47 ` Jens Axboe
2018-06-14 15:29   ` Hannes Reinecke
2018-06-14 15:33     ` Jens Axboe
2018-06-14 16:09       ` Hannes Reinecke
2018-06-14 20:32         ` Adam Manzanares
2018-06-14 20:37           ` Jens Axboe
2018-06-14 20:41             ` Adam Manzanares
2018-06-14 20:47               ` Jens Axboe [this message]
2018-06-14 20:53                 ` Adam Manzanares
2018-06-15  6:06                   ` Hannes Reinecke
2018-06-15  9:23                 ` Mel Gorman
2018-06-15 14:28                   ` Jens Axboe
2018-06-15  7:30       ` Christoph Hellwig
2018-06-15 14:12         ` Jens Axboe
2018-06-15 14:07 ` Bart Van Assche
2018-06-15 16:55   ` Hannes Reinecke
2018-06-15 16:58     ` Bart Van Assche

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=5f21f241-0ae0-ed7a-6935-3ef6e65d0950@kernel.dk \
    --to=axboe@kernel.dk \
    --cc=Adam.Manzanares@wdc.com \
    --cc=hare@suse.com \
    --cc=hare@suse.de \
    --cc=linux-block@vger.kernel.org \
    --cc=mgorman@suse.com \
    /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.