linux-kernel - Re: [PATCH v3 7/9] xen/blkback: separate ring information out of struct xen

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Mon, 19 Oct 2015 18:03:48 +0800
From:	Bob Liu <bob.liu@...cle.com>
To:	Roger Pau Monné <roger.pau@...rix.com>
CC:	xen-devel@...ts.xen.org, david.vrabel@...rix.com,
	linux-kernel@...r.kernel.org, konrad.wilk@...cle.com,
	felipe.franciosi@...rix.com, axboe@...com, hch@...radead.org,
	avanzini.arianna@...il.com, rafal.mielniczuk@...rix.com,
	boris.ostrovsky@...cle.com, jonathan.davies@...rix.com
Subject: Re: [PATCH v3 7/9] xen/blkback: separate ring information out of
 struct xen_blkif


On 10/19/2015 05:36 PM, Roger Pau Monné wrote:
> El 10/10/15 a les 6.08, Bob Liu ha escrit:
>> On 10/05/2015 10:55 PM, Roger Pau Monné wrote:
>>> The same for the pool of persistent grants, it should be per-device and
>>> not per-ring.
>>>
>>> And I think this issue is far worse than the others, because a frontend
>>> might use a persistent grant on different queues, forcing the backend
>>> map the grant several times for each queue, this is not acceptable IMO.
>>>
>>
>> Hi Roger,
>>
>> I realize it would make things complicate if making persistent grant per-device instead of per-queue.
>> Extra locks are required to protect the per-device pool on both blkfront and blkback.
> 
> Yes, I realize this, but without having at least a prototype it's hard
> to tell if contention is going to be a problem or not. We already use a
> red-black tree to store persistent grants, which should be quite fast
> when performing searches.
> 
> IMHO, we are doing things backwards, we should have investigated first
> if using per-device was a problem, and if it indeed was a problem then
> move to per-queue. Using per-device just required adding locks around
> the functions to get/put grants and friends, leaving the data structures
> untouched (per-device).
> 
>> AFAIR, there was a discussion before about dropping persistent grant map at all.
>> The only reason we left this feature was backward compatibility.
>> So that I think we should not complicate xen-block code any more because of a going to be dropped feature.
>>
>> How about disable feature-persistent if multi-queue was used?
> 
> This is not what we have done in the past, also there's no way for
> blkback to tell the frontend that persistent grants and multiqueue
> cannot be used at the same time. Blkback puts all supported features on
> xenstore before knowing anything about the frontend.
> 
> Also, if you want to do it per-queue instead of per-device the limits
> need to be properly adjusted, not just the persistent grants one, but
> also the queue of cached free pages. This also implies that each queue
> it's going to have it's own LRU and purge task.
> 

Okay, then I'll update with a per-device version.

For blkfront there would be two locks used, one for per-device and the other for per-ring.

For blkback, an new lock would be added to protect the red-black tree e.g. in add_persistent_gnt().

-- 
Regards,
-Bob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/