Re: [PATCH 11/11] drm: remove unused and redundant callbacks
On Thu, Jun 22, 2017 at 12:34:36AM +0800, kbuild test robot wrote:
> Hi Peter,
>
> [auto build test ERROR on drm/drm-next]
> [also build test ERROR on next-20170621]
> [cannot apply to v4.12-rc6]
> [if your patch is applied to the wrong git tree, please drop us a note to
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Peter-Rosin/improve-the-fb_setcmap-helper/20170621-205441
> base: git://people.freedesktop.org/~airlied/linux.git drm-next
> config: arm-allmodconfig (attached as .config)
> compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
> wget
> https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=arm
>
> All errors (new ones prefixed by >>):
>
> >> drivers/gpu//drm/armada/armada_fbdev.c:121:2: error: unknown field
> >> 'gamma_set' specified in initializer
> .gamma_set = armada_drm_crtc_gamma_set,
Looks like you've missed at least the armada driver in your conversion?
-Daniel
> ^
> >> drivers/gpu//drm/armada/armada_fbdev.c:121:15: error: initialization from
> >> incompatible pointer type [-Werror=incompatible-pointer-types]
> .gamma_set = armada_drm_crtc_gamma_set,
> ^
>drivers/gpu//drm/armada/armada_fbdev.c:121:15: note: (near initialization
> for 'armada_fb_helper_funcs.fb_probe')
> >> drivers/gpu//drm/armada/armada_fbdev.c:122:2: error: unknown field
> >> 'gamma_get' specified in initializer
> .gamma_get = armada_drm_crtc_gamma_get,
> ^
>drivers/gpu//drm/armada/armada_fbdev.c:122:15: error: initialization from
> incompatible pointer type [-Werror=incompatible-pointer-types]
> .gamma_get = armada_drm_crtc_gamma_get,
> ^
>drivers/gpu//drm/armada/armada_fbdev.c:122:15: note: (near initialization
> for 'armada_fb_helper_funcs.initial_config')
>cc1: some warnings being treated as errors
>
> vim +/gamma_set +121 drivers/gpu//drm/armada/armada_fbdev.c
>
> 96f60e37 Russell King 2012-08-15 115 ret = 1;
> 96f60e37 Russell King 2012-08-15 116 }
> 96f60e37 Russell King 2012-08-15 117 return ret;
> 96f60e37 Russell King 2012-08-15 118 }
> 96f60e37 Russell King 2012-08-15 119
> 3a493879 Thierry Reding 2014-06-27 120 static const struct
> drm_fb_helper_funcs armada_fb_helper_funcs = {
> 96f60e37 Russell King 2012-08-15 @121 .gamma_set =
> armada_drm_crtc_gamma_set,
> 96f60e37 Russell King 2012-08-15 @122 .gamma_get =
> armada_drm_crtc_gamma_get,
> 96f60e37 Russell King 2012-08-15 123 .fb_probe =
> armada_fb_probe,
> 96f60e37 Russell King 2012-08-15 124 };
> 96f60e37 Russell King 2012-08-15 125
>
> :: The code at line 121 was first introduced by commit
> :: 96f60e37dc66091bde8d5de136ff6fda09f2d799 DRM: Armada: Add Armada DRM
> driver
>
> :: TO: Russell King
> :: CC: Russell King
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 01/11] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set
On Wed, Jun 21, 2017 at 11:40:52AM +0200, Peter Rosin wrote: > On 2017-06-21 09:38, Daniel Vetter wrote: > > On Tue, Jun 20, 2017 at 09:25:25PM +0200, Peter Rosin wrote: > >> This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get > >> totally obsolete. > >> > >> I think the gamma_store can end up invalid on error. But the way I read > >> it, that can happen in drm_mode_gamma_set_ioctl as well, so why should > >> this pesky legacy fbdev stuff be any better? > >> > >> drm_fb_helper_save_lut_atomic justs saves the gamma lut for later. However, > >> it saves it to the gamma_store which should already be up to date with what > >> .gamma_get would return and is thus a nop. So, zap it. > > > > Removing drm_fb_helper_save_lut_atomic should be a separate patch I > > think. > > Then 3 patches would be needed, first some hybrid thing that does it the > old way, but also stores the lut in .gamma_store, then the split-out that > removes drm_fb_helper_save_lut_atomic, then whatever is needed to get > to the desired code. I can certainly do that, but do you want me to? Explain that in the commit message and it's fine. > > It's a pre-existing bug, but should we also try to restore the fbdev lut > > in drm_fb_helper_restore_fbdev_mode_unlocked()? Would be yet another bug, > > but might be relevant for your use-case. Just try to run both an fbdev > > application and some kms-native thing, and then SIGKILL the native kms > > app. > > > > But since pre-existing not really required, and probably too much effort. > > Good thing too, because I don't really know my way around this code... Btw I cc'ed you on one of my patches in the fbdev locking series, we might need to do the same legacy vs. atomic split for the new lut code as I did for dpms. The rule with atomic is that you can't do multiple commits under drm_modeset_lock_all, you either have to do one overall atomic commit (preferred) or drop&reacquire locks again. This matters for LUT since you're updating the LUT on all CRTCs, which when using the gamma_set atomic helper would be multiple commits :-/ Using the dpms patch as template it shouldn't be too hard to address that for your patch here too. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 11/11] drm: remove unused and redundant callbacks
Hi Peter,
[auto build test ERROR on drm/drm-next]
[also build test ERROR on next-20170621]
[cannot apply to v4.12-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Peter-Rosin/improve-the-fb_setcmap-helper/20170621-205441
base: git://people.freedesktop.org/~airlied/linux.git drm-next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All errors (new ones prefixed by >>):
>> drivers/gpu//drm/armada/armada_fbdev.c:121:2: error: unknown field
>> 'gamma_set' specified in initializer
.gamma_set = armada_drm_crtc_gamma_set,
^
>> drivers/gpu//drm/armada/armada_fbdev.c:121:15: error: initialization from
>> incompatible pointer type [-Werror=incompatible-pointer-types]
.gamma_set = armada_drm_crtc_gamma_set,
^
drivers/gpu//drm/armada/armada_fbdev.c:121:15: note: (near initialization
for 'armada_fb_helper_funcs.fb_probe')
>> drivers/gpu//drm/armada/armada_fbdev.c:122:2: error: unknown field
>> 'gamma_get' specified in initializer
.gamma_get = armada_drm_crtc_gamma_get,
^
drivers/gpu//drm/armada/armada_fbdev.c:122:15: error: initialization from
incompatible pointer type [-Werror=incompatible-pointer-types]
.gamma_get = armada_drm_crtc_gamma_get,
^
drivers/gpu//drm/armada/armada_fbdev.c:122:15: note: (near initialization
for 'armada_fb_helper_funcs.initial_config')
cc1: some warnings being treated as errors
vim +/gamma_set +121 drivers/gpu//drm/armada/armada_fbdev.c
96f60e37 Russell King 2012-08-15 115 ret = 1;
96f60e37 Russell King 2012-08-15 116 }
96f60e37 Russell King 2012-08-15 117 return ret;
96f60e37 Russell King 2012-08-15 118 }
96f60e37 Russell King 2012-08-15 119
3a493879 Thierry Reding 2014-06-27 120 static const struct
drm_fb_helper_funcs armada_fb_helper_funcs = {
96f60e37 Russell King 2012-08-15 @121 .gamma_set =
armada_drm_crtc_gamma_set,
96f60e37 Russell King 2012-08-15 @122 .gamma_get =
armada_drm_crtc_gamma_get,
96f60e37 Russell King 2012-08-15 123 .fb_probe =
armada_fb_probe,
96f60e37 Russell King 2012-08-15 124 };
96f60e37 Russell King 2012-08-15 125
:: The code at line 121 was first introduced by commit
:: 96f60e37dc66091bde8d5de136ff6fda09f2d799 DRM: Armada: Add Armada DRM
driver
:: TO: Russell King
:: CC: Russell King
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] virtio_scsi: let host do exception handling
virtio_scsi tries to do exception handling after the default
30 seconds timeout expires. However, it's better to let the host
control the timeout, otherwise with a heavy I/O load it is
likely that an abort will also timeout. This leads to fatal
errors like filesystems going offline.
Disable the 'sd' timeout and allow the host to do exception
handling, following the precedent of the storvsc driver.
Hannes has a proposal to introduce timeouts in virtio, but
this provides an immediate solution for stable kernels too.
Reported-by: Douglas Miller
Cc: "James E.J. Bottomley"
Cc: "Martin K. Petersen"
Cc: Hannes Reinecke
Cc: linux-s...@vger.kernel.org
Cc: sta...@vger.kernel.org
Signed-off-by: Paolo Bonzini
---
drivers/scsi/virtio_scsi.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index f8dbfeee6c63..55d344ea6869 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -796,6 +796,16 @@ static int virtscsi_map_queues(struct Scsi_Host *shost)
return blk_mq_virtio_map_queues(&shost->tag_set, vscsi->vdev, 2);
}
+/*
+ * The host guarantees to respond to each command, although I/O latencies might
+ * be higher than on bare meta. Reset the timer unconditionally to give the
+ * host a chance to perform EH.
+ */
+static enum blk_eh_timer_return virtscsi_eh_timed_out(struct scsi_cmnd *scmnd)
+{
+ return BLK_EH_RESET_TIMER;
+}
+
static struct scsi_host_template virtscsi_host_template_single = {
.module = THIS_MODULE,
.name = "Virtio SCSI HBA",
@@ -806,6 +816,7 @@ static struct scsi_host_template
virtscsi_host_template_single = {
.change_queue_depth = virtscsi_change_queue_depth,
.eh_abort_handler = virtscsi_abort,
.eh_device_reset_handler = virtscsi_device_reset,
+ .eh_timed_out = virtscsi_eh_timed_out,
.slave_alloc = virtscsi_device_alloc,
.can_queue = 1024,
@@ -826,6 +837,7 @@ static struct scsi_host_template
virtscsi_host_template_multi = {
.change_queue_depth = virtscsi_change_queue_depth,
.eh_abort_handler = virtscsi_abort,
.eh_device_reset_handler = virtscsi_device_reset,
+ .eh_timed_out = virtscsi_eh_timed_out,
.can_queue = 1024,
.dma_boundary = UINT_MAX,
--
2.13.0
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v11 4/6] mm: function to offer a page block on the free list
On 21.06.2017 14:56, Christian Borntraeger wrote: > On 06/20/2017 06:49 PM, David Hildenbrand wrote: >> On 20.06.2017 18:44, Rik van Riel wrote: >>> On Mon, 2017-06-12 at 07:10 -0700, Dave Hansen wrote: >>> The hypervisor is going to throw away the contents of these pages, right? As soon as the spinlock is released, someone can allocate a page, and put good data in it. What keeps the hypervisor from throwing away good data? >>> >>> That looks like it may be the wrong API, then? >>> >>> We already have hooks called arch_free_page and >>> arch_alloc_page in the VM, which are called when >>> pages are freed, and allocated, respectively. >>> >>> Nitesh Lal (on the CC list) is working on a way >>> to efficiently batch recently freed pages for >>> free page hinting to the hypervisor. >>> >>> If that is done efficiently enough (eg. with >>> MADV_FREE on the hypervisor side for lazy freeing, >>> and lazy later re-use of the pages), do we still >>> need the harder to use batch interface from this >>> patch? >>> >> David's opinion incoming: >> >> No, I think proper free page hinting would be the optimum solution, if >> done right. This would avoid the batch interface and even turn >> virtio-balloon in some sense useless. >> I said "some sense" for a reason. Mainly because other techniques are being worked on that are to fill the holes. > Two reasons why I disagree: > - virtio-balloon is often used as memory hotplug. (e.g. libvirts current/max > memory > uses virtio ballon) I know, while one can argue if this real unplug as there are basically no guarantees (see virtio-mem RFC) it is used by people because there is simply no alternative. Still, for now some people use it for that. > - free page hinting will not allow to shrink the page cache of guests (like a > ballooner does) There are currently some projects ongoing that try to avoid the page cache in the guest completely. -- Thanks, David ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v11 4/6] mm: function to offer a page block on the free list
On 06/20/2017 06:49 PM, David Hildenbrand wrote: > On 20.06.2017 18:44, Rik van Riel wrote: >> On Mon, 2017-06-12 at 07:10 -0700, Dave Hansen wrote: >> >>> The hypervisor is going to throw away the contents of these pages, >>> right? As soon as the spinlock is released, someone can allocate a >>> page, and put good data in it. What keeps the hypervisor from >>> throwing >>> away good data? >> >> That looks like it may be the wrong API, then? >> >> We already have hooks called arch_free_page and >> arch_alloc_page in the VM, which are called when >> pages are freed, and allocated, respectively. >> >> Nitesh Lal (on the CC list) is working on a way >> to efficiently batch recently freed pages for >> free page hinting to the hypervisor. >> >> If that is done efficiently enough (eg. with >> MADV_FREE on the hypervisor side for lazy freeing, >> and lazy later re-use of the pages), do we still >> need the harder to use batch interface from this >> patch? >> > David's opinion incoming: > > No, I think proper free page hinting would be the optimum solution, if > done right. This would avoid the batch interface and even turn > virtio-balloon in some sense useless. > Two reasons why I disagree: - virtio-balloon is often used as memory hotplug. (e.g. libvirts current/max memory uses virtio ballon) - free page hinting will not allow to shrink the page cache of guests (like a ballooner does) ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v11 4/6] mm: function to offer a page block on the free list
On Tue, Jun 20, 2017 at 03:51:00PM -0400, Rik van Riel wrote: > On Tue, 2017-06-20 at 21:26 +0300, Michael S. Tsirkin wrote: > > On Tue, Jun 20, 2017 at 01:29:00PM -0400, Rik van Riel wrote: > > > I agree with that. Let me go into some more detail of > > > what Nitesh is implementing: > > > > > > 1) In arch_free_page, the being-freed page is added > > > to a per-cpu set of freed pages. > > > 2) Once that set is full, arch_free_pages goes into a > > > slow path, which: > > > 2a) Iterates over the set of freed pages, and > > > 2b) Checks whether they are still free, and > > > 2c) Adds the still free pages to a list that is > > > to be passed to the hypervisor, to be MADV_FREEd. > > > 2d) Makes that hypercall. > > > > > > Meanwhile all arch_alloc_pages has to do is make sure it > > > does not allocate a page while it is currently being > > > MADV_FREEd on the hypervisor side. > > > > > > The code Wei is working on looks like it could be > > > suitable for steps (2c) and (2d) above. Nitesh already > > > has code for steps 1 through 2b. > > > > So my question is this: Wei posted these numbers for balloon > > inflation times: > > inflating 7GB of an 8GB idle guest: > > > > 1) allocating pages (6.5%) > > 2) sending PFNs to host (68.3%) > > 3) address translation (6.1%) > > 4) madvise (19%) > > > > It takes about 4126ms for the inflating process to complete. > > > > It seems that this is an excessive amount of time to stay > > under a lock. What are your estimates for Nitesh's work? > > That depends on the batch size used for step > (2c), and is something that we should be able > to tune for decent performance. I am not really sure how you intend to do this. Who will drop and retake the lock? How do you make progress instead of restarting from the beginning? How do you combine multiple pages in a single s/g? All these were issues that Wei's patches solved, granted in a very limited manner (migration-specific) but OTOH without a lot of tuning. > What seems to matter is that things are batched. > There are many ways to achieve that. True, this is what the patches are trying to achieve. So far this approach was the 1st more or less workable way do achieve that, previous ones got us nowhere. > -- > All rights reversed ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC] virtio-mem: paravirtualized memory
On 21.06.2017 13:08, Stefan Hajnoczi wrote:
> On Mon, Jun 19, 2017 at 12:26:52PM +0200, David Hildenbrand wrote:
>> On 19.06.2017 12:08, Stefan Hajnoczi wrote:
>>> On Fri, Jun 16, 2017 at 04:20:02PM +0200, David Hildenbrand wrote:
Important restrictions of this concept:
- Guests without a virtio-mem guest driver can't see that memory.
- We will always require some boot memory that cannot get unplugged.
Also, virtio-mem memory (as all other hotplugged memory) cannot become
DMA memory under Linux. So the boot memory also defines the amount of
DMA memory.
>>>
>>> I didn't know that hotplug memory cannot become DMA memory.
>>>
>>> Ouch. Zero-copy disk I/O with O_DIRECT and network I/O with virtio-net
>>> won't be possible.
>>>
>>> When running an application that uses O_DIRECT file I/O this probably
>>> means we now have 2 copies of pages in memory: 1. in the application and
>>> 2. in the kernel page cache.
>>>
>>> So this increases pressure on the page cache and reduces performance :(.
>>>
>>> Stefan
>>>
>>
>> arch/x86/mm/init_64.c:
>>
>> /*
>> * Memory is added always to NORMAL zone. This means you will never get
>> * additional DMA/DMA32 memory.
>> */
>> int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
>> {
>>
>> The is for sure something to work on in the future. Until then, base
>> memory of 3.X GB should be sufficient, right?
>
> I'm not sure that helps because applications typically don't control
> where their buffers are located?
Okay, let me try to explain what is going on here (no expert, please
someone correct me if I am wrong).
There is a difference between DMA and DMA memory in Linux. DMA memory is
simply memory with special addresses. DMA is the general technique of a
device directly copying data to ram, bypassing the CPU.
ZONE_DMA contains all* memory < 16MB
ZONE_DMA32 contains all* memory < 4G
* meaning available on boot via a820 map, not hotplugged.
So memory from these zones can be used by devices that can only deal
with 24bit/32bit addresses.
Hotplugged memory is never added to the ZONE_DMA/DMA32, but to
ZONE_NORMAL. That means, kmalloc(.., GFP_DMA will) not be able to use
hotplugged memory. Say you have 1GB of main storage and hotplug 1G (on
address 1G). This memory will not be available in the ZONE_DMA, although
below 4g.
Memory in ZONE_NORMAL is used for ordinary kmalloc(), so all these
memory can be used to do DMA, but you are not guaranteed to get 32bit
capable addresses. I pretty much assume that virtio-net can deal with
64bit addresses.
My understanding of O_DIRECT:
The user space buffers (O_DIRECT) is directly used to do DMA. This will
work just fine as long as the device can deal with 64bit addresses. I
guess this is the case for virtio-net, otherwise there would be the
exact same problem already without virtio-mem.
Summary:
virtio-mem memory can be used for DMA, it will simply not be added to
ZONE_DMA/DMA32 and therefore won't be available for kmalloc(...,
GFP_DMA). This should work just fine with O_DIRECT as before.
If necessary, we could try to add memory to the ZONE_DMA later on,
however for now I would rate this a minor problem. By simply using 3.X
GB of base memory, basically all memory that could go to ZONE_DMA/DMA32
already is in these zones without virtio-mem.
Thanks!
>
> Stefan
>
--
Thanks,
David
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [virtio-dev] Re: [PATCH v11 6/6] virtio-balloon: VIRTIO_BALLOON_F_CMD_VQ
On Wed, Jun 21, 2017 at 11:28:00AM +0800, Wei Wang wrote:
> On 06/21/2017 12:18 AM, Michael S. Tsirkin wrote:
> > On Fri, Jun 09, 2017 at 06:41:41PM +0800, Wei Wang wrote:
> > > - if (!virtqueue_indirect_desc_table_add(vq, desc, num)) {
> > > + if (!virtqueue_indirect_desc_table_add(vq, desc, *num)) {
> > > virtqueue_kick(vq);
> > > - wait_event(vb->acked, virtqueue_get_buf(vq, &len));
> > > - vb->balloon_page_chunk.chunk_num = 0;
> > > + if (busy_wait)
> > > + while (!virtqueue_get_buf(vq, &len) &&
> > > +!virtqueue_is_broken(vq))
> > > + cpu_relax();
> > > + else
> > > + wait_event(vb->acked, virtqueue_get_buf(vq, &len));
> >
> > This is something I didn't previously notice.
> > As you always keep a single buffer in flight, you do not
> > really need indirect at all. Just add all descriptors
> > in the ring directly, then kick.
> >
> > E.g.
> > virtqueue_add_first
> > virtqueue_add_next
> > virtqueue_add_last
> >
> > ?
> >
> > You also want a flag to avoid allocations but there's no need to do it
> > per descriptor, set it on vq.
> >
>
> Without using the indirect table, I'm thinking about changing to use
> the standard sg (i.e. struct scatterlist), instead of vring_desc, so that
> we don't need to modify or add any new functions of virtqueue_add().
>
> In this case, we will kmalloc an array of sgs in probe(), and we can add
> the sgs one by one to the vq, which won't trigger the allocation of an
> indirect table inside virtqueue_add(), and then kick when all are added.
>
> Best,
> Wei
And allocate headers too? This can work. API extensions aren't
necessarily a bad idea though. The API I suggest above is preferable
for the simple reason that it can work without INDIRECT flag
support in hypervisor.
I wonder which APIs would Nitesh find useful.
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [RFC] virtio-mem: paravirtualized memory
On Mon, Jun 19, 2017 at 12:26:52PM +0200, David Hildenbrand wrote:
> On 19.06.2017 12:08, Stefan Hajnoczi wrote:
> > On Fri, Jun 16, 2017 at 04:20:02PM +0200, David Hildenbrand wrote:
> >> Important restrictions of this concept:
> >> - Guests without a virtio-mem guest driver can't see that memory.
> >> - We will always require some boot memory that cannot get unplugged.
> >> Also, virtio-mem memory (as all other hotplugged memory) cannot become
> >> DMA memory under Linux. So the boot memory also defines the amount of
> >> DMA memory.
> >
> > I didn't know that hotplug memory cannot become DMA memory.
> >
> > Ouch. Zero-copy disk I/O with O_DIRECT and network I/O with virtio-net
> > won't be possible.
> >
> > When running an application that uses O_DIRECT file I/O this probably
> > means we now have 2 copies of pages in memory: 1. in the application and
> > 2. in the kernel page cache.
> >
> > So this increases pressure on the page cache and reduces performance :(.
> >
> > Stefan
> >
>
> arch/x86/mm/init_64.c:
>
> /*
> * Memory is added always to NORMAL zone. This means you will never get
> * additional DMA/DMA32 memory.
> */
> int arch_add_memory(int nid, u64 start, u64 size, bool for_device)
> {
>
> The is for sure something to work on in the future. Until then, base
> memory of 3.X GB should be sufficient, right?
I'm not sure that helps because applications typically don't control
where their buffers are located?
Stefan
signature.asc
Description: PGP signature
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [Qemu-devel] [PATCH v11 4/6] mm: function to offer a page block on the free list
On 06/21/2017 01:29 AM, Rik van Riel wrote: On Tue, 2017-06-20 at 18:49 +0200, David Hildenbrand wrote: On 20.06.2017 18:44, Rik van Riel wrote: Nitesh Lal (on the CC list) is working on a way to efficiently batch recently freed pages for free page hinting to the hypervisor. If that is done efficiently enough (eg. with MADV_FREE on the hypervisor side for lazy freeing, and lazy later re-use of the pages), do we still need the harder to use batch interface from this patch? David's opinion incoming: No, I think proper free page hinting would be the optimum solution, if done right. This would avoid the batch interface and even turn virtio-balloon in some sense useless. I agree with that. Let me go into some more detail of what Nitesh is implementing: 1) In arch_free_page, the being-freed page is added to a per-cpu set of freed pages. I got some questions here: 1. Are the pages managed one by one on the per-CPU set? For example, when there are 2 adjacent pages, are they still put as two nodes on the per-CPU list? or the buddy algorithm will be re-implemented on the per-CPU list as well? 2. Looks like this will be added to the common free function. Normally, people may not need the free page hint, do they need to carry the added burden? 2) Once that set is full, arch_free_pages goes into a slow path, which: 2a) Iterates over the set of freed pages, and 2b) Checks whether they are still free, and The pages that have been double checked as "free" pages here and added to the list for the hypervisor can also be immediately used. 2c) Adds the still free pages to a list that is to be passed to the hypervisor, to be MADV_FREEd. 2d) Makes that hypercall. Meanwhile all arch_alloc_pages has to do is make sure it does not allocate a page while it is currently being MADV_FREEd on the hypervisor side. Is this proposed to replace the balloon driver? The code Wei is working on looks like it could be suitable for steps (2c) and (2d) above. Nitesh already has code for steps 1 through 2b. May I know the advantages of the added steps? Thanks. Best, Wei ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [Nouveau] [PATCH 01/11] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set
On Wed, Jun 21, 2017 at 04:59:31PM +0900, Michel Dänzer wrote:
> On 21/06/17 04:38 PM, Daniel Vetter wrote:
> > On Tue, Jun 20, 2017 at 09:25:25PM +0200, Peter Rosin wrote:
> >> This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
> >> totally obsolete.
> >>
> >> I think the gamma_store can end up invalid on error. But the way I read
> >> it, that can happen in drm_mode_gamma_set_ioctl as well, so why should
> >> this pesky legacy fbdev stuff be any better?
> >>
> >> drm_fb_helper_save_lut_atomic justs saves the gamma lut for later. However,
> >> it saves it to the gamma_store which should already be up to date with what
> >> .gamma_get would return and is thus a nop. So, zap it.
> >
> > Removing drm_fb_helper_save_lut_atomic should be a separate patch I
> > think.
> >
> >> Signed-off-by: Peter Rosin
>
> [...]
>
> >> @@ -1167,50 +1150,6 @@ void drm_fb_helper_set_suspend_unlocked(struct
> >> drm_fb_helper *fb_helper,
> >> }
> >> EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked);
> >>
> >> -static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
> >> - u16 blue, u16 regno, struct fb_info *info)
> >> -{
> >> - struct drm_fb_helper *fb_helper = info->par;
> >> - struct drm_framebuffer *fb = fb_helper->fb;
> >> -
> >> - if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
> >
> > This case here seems gone, and it was also the pièce de résistance when I
> > tried tackling fbdev lut support. As far as I understand this stuff we do
> > not support FB_VISUAL_TRUECOLOR palette, and all that bitshifting here is
> > pointless. But I'm honestly not really clear.
> >
> > I think removing this case should also be a separate patch, and I'd very
> > much prefer that someone with some fbdev-clue would ack it.
>
> No need for anyone with fbdev-clue, just run fbset -i. :) fbcon uses a
> TRUECOLOR visual at depths > 8.
Then I understand even less what exactly this code does ... Should we keep
it? Is it just pure cargo-cult? Only needed when we'd change the color
depth of the fbdev buffer, which we never do at runtime?
> > It's a pre-existing bug, but should we also try to restore the fbdev lut
> > in drm_fb_helper_restore_fbdev_mode_unlocked()? Would be yet another bug,
> > but might be relevant for your use-case. Just try to run both an fbdev
> > application and some kms-native thing, and then SIGKILL the native kms
> > app.
> >
> > But since pre-existing not really required, and probably too much effort.
>
> I suspect something like that indeed needs to be done though. E.g.
> killing Xorg results in fbcon continuing to use the LUT set by Xorg.
Ok, the only trouble with that is atomic kms locking, which requires that
we can only do 1 commit per lock-holding time, and also
drm_modeset_lock_all is an evil hack and needs to be phased out. Two
solutions:
- We just update the lut after we've dropped the locks again in
restore_fbdev_mode.
- We need both a legacy path, in restore_fbdev_mode_legacy, and an atomic
path in restore_fbdev_mode_atomic. The latter cannot use the gamme_set
callback, since that would call drm_atomic_helper_legacy_gamma_set for
atomic drivers, which would result in two calls to drm_atomic_commit in
one critical sections. Instead that needs to open-code the logic in
drm_atomic_helper_legacy_gamma_set and integrate it into the overall
fbdev restore commit.
The 2nd option is a bit more typing, but much cleaner. It also fits better
into some of the refactoring plans I have (I want to get rid of
drm_modeset_lock_all in the fbdev emulation). And has the benefit of
giving drivers a bit more complex fbdev emulation atomic commits, which
would be good for testing I think.
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 00/11] improve the fb_setcmap helper
On Tue, Jun 20, 2017 at 09:25:24PM +0200, Peter Rosin wrote: > Hi! > > While trying to get CLUT support for the atmel_hlcdc driver, and > specifically for the emulated fbdev interface, I received some > push-back that my feeble in-driver attempts should be solved > by the core. This is my attempt to do it right. > > Boris and Daniel, was this approximately what you had in mind? Yeah, this is awesome. I tried to do it a few times myself, but always failed (also due to lack of real use-case on my side). > I have obviously not tested all of this with more than a compile, > but the first patch is enough to make the atmel-hlcdc driver > do what I need. The rest is just lots of removals and cleanup > made possible by the improved core. If it works for you it's imo good enough. Not sure anyone else really cares about fbdev lut support at all. I have a few comments on the first patch, but once that's sorted, and once we have given driver maintainers enough time to ack I think I'll merge the entire pile into drm-misc. Nice work, thanks for doing it. Cheers, Daniel > Please test, I would not be surprised if I have fouled up some > bit-manipulation somewhere in this mostly mechanical change... > > Cheers, > peda > > Peter Rosin (11): > drm/fb-helper: do a generic fb_setcmap helper in terms of crtc > .gamma_set > drm: amd: remove dead code and pointless local lut storage > drm: ast: remove dead code and pointless local lut storage > drm: cirrus: remove dead code and pointless local lut storage > dmr: gma500: remove dead code and pointless local lut storage > drm: i915: remove dead code and pointless local lut storage > drm: mgag200: remove dead code and pointless local lut storage > drm: nouveau: remove dead code and pointless local lut storage > drm: radeon: remove dead code and pointless local lut storage > drm: stm: remove dead code and pointless local lut storage > drm: remove unused and redundant callbacks > > drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 24 - > drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h| 1 - > drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 27 ++ > drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 27 ++ > drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 27 ++ > drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 27 ++ > drivers/gpu/drm/amd/amdgpu/dce_virtual.c| 23 - > drivers/gpu/drm/ast/ast_drv.h | 1 - > drivers/gpu/drm/ast/ast_fb.c| 20 - > drivers/gpu/drm/ast/ast_mode.c | 26 ++ > drivers/gpu/drm/cirrus/cirrus_drv.h | 8 -- > drivers/gpu/drm/cirrus/cirrus_fbdev.c | 2 - > drivers/gpu/drm/cirrus/cirrus_mode.c| 71 --- > drivers/gpu/drm/drm_fb_helper.c | 131 > +--- > drivers/gpu/drm/gma500/framebuffer.c| 22 - > drivers/gpu/drm/gma500/gma_display.c| 32 +++ > drivers/gpu/drm/gma500/psb_intel_display.c | 7 +- > drivers/gpu/drm/gma500/psb_intel_drv.h | 1 - > drivers/gpu/drm/i915/intel_drv.h| 1 - > drivers/gpu/drm/i915/intel_fbdev.c | 31 --- > drivers/gpu/drm/mgag200/mgag200_drv.h | 5 -- > drivers/gpu/drm/mgag200/mgag200_fb.c| 2 - > drivers/gpu/drm/mgag200/mgag200_mode.c | 62 - > drivers/gpu/drm/nouveau/dispnv04/crtc.c | 26 ++ > drivers/gpu/drm/nouveau/nouveau_crtc.h | 3 - > drivers/gpu/drm/nouveau/nouveau_fbcon.c | 22 - > drivers/gpu/drm/nouveau/nv50_display.c | 39 +++-- > drivers/gpu/drm/radeon/atombios_crtc.c | 1 - > drivers/gpu/drm/radeon/radeon_connectors.c | 7 +- > drivers/gpu/drm/radeon/radeon_display.c | 71 ++- > drivers/gpu/drm/radeon/radeon_fb.c | 2 - > drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 1 - > drivers/gpu/drm/stm/ltdc.c | 12 --- > drivers/gpu/drm/stm/ltdc.h | 1 - > include/drm/drm_fb_helper.h | 32 --- > include/drm/drm_modeset_helper_vtables.h| 16 > 36 files changed, 171 insertions(+), 640 deletions(-) > > -- > 2.1.4 > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 01/11] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set
On Tue, Jun 20, 2017 at 09:25:25PM +0200, Peter Rosin wrote:
> This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
> totally obsolete.
>
> I think the gamma_store can end up invalid on error. But the way I read
> it, that can happen in drm_mode_gamma_set_ioctl as well, so why should
> this pesky legacy fbdev stuff be any better?
>
> drm_fb_helper_save_lut_atomic justs saves the gamma lut for later. However,
> it saves it to the gamma_store which should already be up to date with what
> .gamma_get would return and is thus a nop. So, zap it.
Removing drm_fb_helper_save_lut_atomic should be a separate patch I
think.
> Signed-off-by: Peter Rosin
> ---
> drivers/gpu/drm/drm_fb_helper.c | 131
>
> 1 file changed, 40 insertions(+), 91 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 574af01..cc2d55d 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -229,22 +229,6 @@ int drm_fb_helper_remove_one_connector(struct
> drm_fb_helper *fb_helper,
> }
> EXPORT_SYMBOL(drm_fb_helper_remove_one_connector);
>
> -static void drm_fb_helper_save_lut_atomic(struct drm_crtc *crtc, struct
> drm_fb_helper *helper)
> -{
> - uint16_t *r_base, *g_base, *b_base;
> - int i;
> -
> - if (helper->funcs->gamma_get == NULL)
> - return;
> -
> - r_base = crtc->gamma_store;
> - g_base = r_base + crtc->gamma_size;
> - b_base = g_base + crtc->gamma_size;
> -
> - for (i = 0; i < crtc->gamma_size; i++)
> - helper->funcs->gamma_get(crtc, &r_base[i], &g_base[i],
> &b_base[i], i);
> -}
> -
> static void drm_fb_helper_restore_lut_atomic(struct drm_crtc *crtc)
> {
> uint16_t *r_base, *g_base, *b_base;
> @@ -285,7 +269,6 @@ int drm_fb_helper_debug_enter(struct fb_info *info)
> if (drm_drv_uses_atomic_modeset(mode_set->crtc->dev))
> continue;
>
> - drm_fb_helper_save_lut_atomic(mode_set->crtc, helper);
> funcs->mode_set_base_atomic(mode_set->crtc,
> mode_set->fb,
> mode_set->x,
> @@ -1167,50 +1150,6 @@ void drm_fb_helper_set_suspend_unlocked(struct
> drm_fb_helper *fb_helper,
> }
> EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked);
>
> -static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
> - u16 blue, u16 regno, struct fb_info *info)
> -{
> - struct drm_fb_helper *fb_helper = info->par;
> - struct drm_framebuffer *fb = fb_helper->fb;
> -
> - if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
This case here seems gone, and it was also the pièce de résistance when I
tried tackling fbdev lut support. As far as I understand this stuff we do
not support FB_VISUAL_TRUECOLOR palette, and all that bitshifting here is
pointless. But I'm honestly not really clear.
I think removing this case should also be a separate patch, and I'd very
much prefer that someone with some fbdev-clue would ack it.
> - u32 *palette;
> - u32 value;
> - /* place color in psuedopalette */
> - if (regno > 16)
> - return -EINVAL;
> - palette = (u32 *)info->pseudo_palette;
> - red >>= (16 - info->var.red.length);
> - green >>= (16 - info->var.green.length);
> - blue >>= (16 - info->var.blue.length);
> - value = (red << info->var.red.offset) |
> - (green << info->var.green.offset) |
> - (blue << info->var.blue.offset);
> - if (info->var.transp.length > 0) {
> - u32 mask = (1 << info->var.transp.length) - 1;
> -
> - mask <<= info->var.transp.offset;
> - value |= mask;
> - }
> - palette[regno] = value;
> - return 0;
> - }
> -
> - /*
> - * The driver really shouldn't advertise pseudo/directcolor
> - * visuals if it can't deal with the palette.
> - */
> - if (WARN_ON(!fb_helper->funcs->gamma_set ||
> - !fb_helper->funcs->gamma_get))
> - return -EINVAL;
> -
> - WARN_ON(fb->format->cpp[0] != 1);
> -
> - fb_helper->funcs->gamma_set(crtc, red, green, blue, regno);
> -
> - return 0;
> -}
> -
> /**
> * drm_fb_helper_setcmap - implementation for &fb_ops.fb_setcmap
> * @cmap: cmap to set
> @@ -1220,51 +1159,61 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap,
> struct fb_info *info)
> {
> struct drm_fb_helper *fb_helper = info->par;
> struct drm_device *dev = fb_helper->dev;
> - const struct drm_crtc_helper_funcs *crtc_funcs;
> - u16 *red, *green, *blue, *transp;
> + struct drm_modeset_acquire_ctx ctx;
> struct drm_crtc *crtc;
> - int i, j, rc = 0;
> - int
