Re: [PATCH 1/2] virtio-fs: limit number of request queues
On 5/28/24 5:05 PM, Peter-Jan Gootzen wrote: > Hi Jingbo, > > On Tue, 2024-05-28 at 16:40 +0800, Jingbo Xu wrote: >> External email: Use caution opening links or attachments >> >> >> Hi Peter-Jan, >> >> Thanks for the amazing work! Glad to see it is upstreamed. We are >> also >> planning distribute virtiofs on DPU. > Great to hear that virtio-fs is getting more attention with this > powerful approach :) > >> >> BTW I have some questions when reading the patch, appreciate if you >> could give a hint. >> >> >> On 5/1/24 11:38 PM, Peter-Jan Gootzen wrote: >>> Virtio-fs devices might allocate significant resources to virtio >>> queues >>> such as CPU cores that busy poll on the queue. The device indicates >>> how >>> many request queues it can support and the driver should initialize >>> the >>> number of queues that they want to utilize. >> >> I'm okay with truncating the num_request_queues to nr_cpu_ids if the >> number of the available queues exceeds nr_cpu_ids, as generally 1:1 >> mapping between cpus and queues is enough and we can not make use more >> queues in this case. >> >> I just don't understand the above commit message as how this relates >> to >> the resource footprint at the device side. When the the number of the >> queues actually used by the driver (e.g. nr_cpu_ids is 6) is less than >> the amount (e.g. 8) that is advertised by the driver, will the device >> side reclaim the resources allocated for the remaining unused queues? >> The driver has no way notifying how many queues it actually utilizes. > > The device resources allocated to queues is device implementation > specific. So the cost of the driver creating more queues than it will > ultimately use, is dependent on how the specific device handles these > unsused queues. > A smart device could at some point decide to spend less polling > resources on a queue if it sees that it is unused (reclaiming the > resources as you put it). > > But in my opinion, the driver should not allocate more than it will use. > The new scheme tries to map every core to a queue, so we know that we > will not use >nr_cpu_ids queues. Okay got it. Many thanks for the reply. -- Thanks, Jingbo
Re: [PATCH 1/2] virtio-fs: limit number of request queues
Hi Jingbo, On Tue, 2024-05-28 at 16:40 +0800, Jingbo Xu wrote: > External email: Use caution opening links or attachments > > > Hi Peter-Jan, > > Thanks for the amazing work! Glad to see it is upstreamed. We are > also > planning distribute virtiofs on DPU. Great to hear that virtio-fs is getting more attention with this powerful approach :) > > BTW I have some questions when reading the patch, appreciate if you > could give a hint. > > > On 5/1/24 11:38 PM, Peter-Jan Gootzen wrote: > > Virtio-fs devices might allocate significant resources to virtio > > queues > > such as CPU cores that busy poll on the queue. The device indicates > > how > > many request queues it can support and the driver should initialize > > the > > number of queues that they want to utilize. > > I'm okay with truncating the num_request_queues to nr_cpu_ids if the > number of the available queues exceeds nr_cpu_ids, as generally 1:1 > mapping between cpus and queues is enough and we can not make use more > queues in this case. > > I just don't understand the above commit message as how this relates > to > the resource footprint at the device side. When the the number of the > queues actually used by the driver (e.g. nr_cpu_ids is 6) is less than > the amount (e.g. 8) that is advertised by the driver, will the device > side reclaim the resources allocated for the remaining unused queues? > The driver has no way notifying how many queues it actually utilizes. The device resources allocated to queues is device implementation specific. So the cost of the driver creating more queues than it will ultimately use, is dependent on how the specific device handles these unsused queues. A smart device could at some point decide to spend less polling resources on a queue if it sees that it is unused (reclaiming the resources as you put it). But in my opinion, the driver should not allocate more than it will use. The new scheme tries to map every core to a queue, so we know that we will not use >nr_cpu_ids queues. - Peter-Jan
Re: [PATCH 1/2] virtio-fs: limit number of request queues
Hi Peter-Jan, Thanks for the amazing work! Glad to see it is upstreamed. We are also planning distribute virtiofs on DPU. BTW I have some questions when reading the patch, appreciate if you could give a hint. On 5/1/24 11:38 PM, Peter-Jan Gootzen wrote: > Virtio-fs devices might allocate significant resources to virtio queues > such as CPU cores that busy poll on the queue. The device indicates how > many request queues it can support and the driver should initialize the > number of queues that they want to utilize. I'm okay with truncating the num_request_queues to nr_cpu_ids if the number of the available queues exceeds nr_cpu_ids, as generally 1:1 mapping between cpus and queues is enough and we can not make use more queues in this case. I just don't understand the above commit message as how this relates to the resource footprint at the device side. When the the number of the queues actually used by the driver (e.g. nr_cpu_ids is 6) is less than the amount (e.g. 8) that is advertised by the driver, will the device side reclaim the resources allocated for the remaining unused queues? The driver has no way notifying how many queues it actually utilizes. > > In this patch we limit the number of initialized request queues to the > number of CPUs, to limit the resource consumption on the device-side > and to prepare for the upcoming multi-queue patch. > > Signed-off-by: Peter-Jan Gootzen > Signed-off-by: Yoray Zack > Suggested-by: Max Gurtovoy > Reviewed-by: Max Gurtovoy > --- > fs/fuse/virtio_fs.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c > index 322af827a232..2745fa484dcb 100644 > --- a/fs/fuse/virtio_fs.c > +++ b/fs/fuse/virtio_fs.c > @@ -751,6 +751,9 @@ static int virtio_fs_setup_vqs(struct virtio_device *vdev, > if (fs->num_request_queues == 0) > return -EINVAL; > > + /* Truncate nr of request queues to nr_cpu_id */ > + fs->num_request_queues = min_t(unsigned int, fs->num_request_queues, > + nr_cpu_ids); > fs->nvqs = VQ_REQUEST + fs->num_request_queues; > fs->vqs = kcalloc(fs->nvqs, sizeof(fs->vqs[VQ_HIPRIO]), GFP_KERNEL); > if (!fs->vqs) -- Thanks, Jingbo
Re: [PATCH 1/2] virtio-fs: limit number of request queues
On Wed, May 01, 2024 at 05:38:16PM +0200, Peter-Jan Gootzen wrote: > Virtio-fs devices might allocate significant resources to virtio queues > such as CPU cores that busy poll on the queue. The device indicates how > many request queues it can support and the driver should initialize the > number of queues that they want to utilize. > > In this patch we limit the number of initialized request queues to the > number of CPUs, to limit the resource consumption on the device-side > and to prepare for the upcoming multi-queue patch. > > Signed-off-by: Peter-Jan Gootzen > Signed-off-by: Yoray Zack > Suggested-by: Max Gurtovoy > Reviewed-by: Max Gurtovoy > --- > fs/fuse/virtio_fs.c | 3 +++ > 1 file changed, 3 insertions(+) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
[PATCH 1/2] virtio-fs: limit number of request queues
Virtio-fs devices might allocate significant resources to virtio queues such as CPU cores that busy poll on the queue. The device indicates how many request queues it can support and the driver should initialize the number of queues that they want to utilize. In this patch we limit the number of initialized request queues to the number of CPUs, to limit the resource consumption on the device-side and to prepare for the upcoming multi-queue patch. Signed-off-by: Peter-Jan Gootzen Signed-off-by: Yoray Zack Suggested-by: Max Gurtovoy Reviewed-by: Max Gurtovoy --- fs/fuse/virtio_fs.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c index 322af827a232..2745fa484dcb 100644 --- a/fs/fuse/virtio_fs.c +++ b/fs/fuse/virtio_fs.c @@ -751,6 +751,9 @@ static int virtio_fs_setup_vqs(struct virtio_device *vdev, if (fs->num_request_queues == 0) return -EINVAL; + /* Truncate nr of request queues to nr_cpu_id */ + fs->num_request_queues = min_t(unsigned int, fs->num_request_queues, + nr_cpu_ids); fs->nvqs = VQ_REQUEST + fs->num_request_queues; fs->vqs = kcalloc(fs->nvqs, sizeof(fs->vqs[VQ_HIPRIO]), GFP_KERNEL); if (!fs->vqs) -- 2.34.1
