[Spice-commits] client/glz_decoder_window.cpp client/glz_decoder_window.h client/red_client.cpp

Hans de Goede jwrdegoede at kemper.freedesktop.org
Mon Oct 11 04:24:19 PDT 2010


 client/glz_decoder_window.cpp |   35 ++---------------------------------
 client/glz_decoder_window.h   |    6 +-----
 client/red_client.cpp         |    3 +--
 3 files changed, 4 insertions(+), 40 deletions(-)

New commits:
commit 3a4051d7ce70914e5cb88496a6a2510ede793325
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Sat Oct 9 22:13:58 2010 +0200

    spicec: Do not try to do accounting of pci memory
    
    Without this patch spicec reproducely hangs in
    GlzDecoderWindow::pre_decode_update_window().
    
    When GlzDecoderWindow::will_overflow() returns true,
    GlzDecoderWindow::pre_decode_update_window(),
    waits for a call to GlzDecoderWindow::post_decode()
    to free up some memory
    
    This happens even though there still is pci memory
    available (otherwise the driver would not have
    been able to send an image to decode in the first
    place).
    
    The GlzDecoderWindow::post_decode() call never happens
    as the server is waiting for a reply to the decode
    of the hanging image, causing the client to hang
    for ever.
    
    This patch fixes this by simply removing the
    "attempted" pci memory accounting. As there is no
    need for that, as the driver already must keep
    track of pci memory usage.
    
    I've verified that both the old and new Xorg drivers
    take care of not overusing the pci memory themselves
    I would expect the same to be true for the windows
    driver.
    
    Note the calculating of the glz_window_size in
    red_client.cpp cannot be removed as the calculated
    value is send as part of the SpiceMsgcDisplayInit on
    connect.

diff --git a/client/glz_decoder_window.cpp b/client/glz_decoder_window.cpp
index 24dfce3..ab081f8 100644
--- a/client/glz_decoder_window.cpp
+++ b/client/glz_decoder_window.cpp
@@ -21,21 +21,12 @@
 #include "utils.h"
 
 #define INIT_IMAGES_CAPACITY 100
-#define WIN_OVERFLOW_FACTOR 1.5
 #define WIN_REALLOC_FACTOR 1.5
 
-GlzDecoderWindow::GlzDecoderWindow(int pixels_capacity, GlzDecoderDebug &debug_calls)
-    : _pixels_capacity (pixels_capacity)
-    , _aborting (false)
+GlzDecoderWindow::GlzDecoderWindow(GlzDecoderDebug &debug_calls)
+    : _aborting (false)
     , _debug_calls (debug_calls)
 {
-    if (_pixels_capacity > LZ_MAX_WINDOW_SIZE) {
-        std::string erro_str;
-        string_printf(erro_str, "Glz Window capacity exceeds the limit %d",
-                      _pixels_capacity);
-        _debug_calls.error(erro_str);
-    }
-
     _images_capacity = INIT_IMAGES_CAPACITY;
     _images = new  GlzDecodedImage*[_images_capacity];
     if (!_images) {
@@ -105,19 +96,6 @@ void GlzDecoderWindow::clear()
     init();
 }
 
-void GlzDecoderWindow::set_pixels_capacity(int pixels_capacity)
-{
-    Lock lock(_win_modifiers_mutex);
-
-    if (pixels_capacity > LZ_MAX_WINDOW_SIZE) {
-        std::string erro_str;
-        string_printf(erro_str, "Glz Window capacity exceeds the limit %d",
-                      pixels_capacity);
-        _debug_calls.error(erro_str);
-    }
-    _pixels_capacity = pixels_capacity;
-}
-
 void GlzDecoderWindow::init()
 {
     _missing_list.clear();
@@ -127,7 +105,6 @@ void GlzDecoderWindow::init()
     _head_idx = 0;
     _tail_image_id = 0;
     _n_images = 1;
-    _n_pixels = 0;
 }
 
 void GlzDecoderWindow::release_images()
@@ -146,18 +123,12 @@ inline bool GlzDecoderWindow::is_empty()
     return (!_n_images);
 }
 
-/* approximated overflow. Considers only the size that currently occupies the window and
-   not the size of the missing images. TODO: consider other measures */
 inline bool GlzDecoderWindow::will_overflow(uint64_t image_id, uint64_t relative_head_id)
 {
     if (image_id <= _tail_image_id) {
         return false;
     }
 
-    if (_n_pixels > (WIN_OVERFLOW_FACTOR * _pixels_capacity)) {
-        return true;
-    }
-
     if (!_missing_list.empty() && (_missing_list.front() < relative_head_id)) {
         // two non overlapping windows
         return true;
@@ -276,7 +247,6 @@ void GlzDecoderWindow::add_decoded_image(GlzDecodedImage *image)
     Lock lock(_new_image_mutex);
     GLZ_ASSERT(_debug_calls, image->get_id() <= _tail_image_id);
     _images[calc_image_win_idx(image->get_id())] = image;
-    _n_pixels += image->get_size();
     _new_image_cond.notify_all();
 }
 
@@ -339,7 +309,6 @@ inline void GlzDecoderWindow::remove_head(uint64_t new_head_image_id)
 
     for (int i = 0; i < n_images_remove; i++) {
         int index = (_head_idx + i) % _images_capacity;
-        _n_pixels -= _images[index]->get_size();
         delete _images[index];
         _images[index] = NULL;
     }
diff --git a/client/glz_decoder_window.h b/client/glz_decoder_window.h
index a2848bb..e061c73 100644
--- a/client/glz_decoder_window.h
+++ b/client/glz_decoder_window.h
@@ -33,7 +33,7 @@ typedef int DecodedImageWinId;
 
 class GlzDecoderWindow {
 public:
-    GlzDecoderWindow(int pixels_capacity, GlzDecoderDebug &debug_calls);
+    GlzDecoderWindow(GlzDecoderDebug &debug_calls);
     virtual ~GlzDecoderWindow();
 
     DecodedImageWinId pre_decode(uint64_t image_id, uint64_t relative_head_id);
@@ -50,8 +50,6 @@ public:
     /* NOTE - clear mustn't be called if the window is currently used by a decoder*/
     void clear();
 
-    void set_pixels_capacity(int pixels_capacity);
-
 private:
     void wait_for_image(int index);
     void add_image(GlzDecodedImage *image);
@@ -75,14 +73,12 @@ private:
     void release_images();
 
 private:
-    int _pixels_capacity;
     GlzDecodedImage **_images; // cyclic window
     int _head_idx;            // index in images array (not image id)
     uint64_t _tail_image_id;
     int _images_capacity;
     int _n_images;            // _n_images counts all the images in
                               // the window, including the missing ones
-    uint64_t _n_pixels;
 
     std::list<uint64_t> _missing_list;
 
diff --git a/client/red_client.cpp b/client/red_client.cpp
index 61974b5..b9e1421 100644
--- a/client/red_client.cpp
+++ b/client/red_client.cpp
@@ -367,7 +367,7 @@ RedClient::RedClient(Application& application)
     , _agent_caps_size(0)
     , _agent_caps(NULL)
     , _migrate (*this)
-    , _glz_window (0, _glz_debug)
+    , _glz_window (_glz_debug)
 {
     Platform::set_clipboard_listener(this);
     MainChannelLoop* message_loop = static_cast<MainChannelLoop*>(get_message_handler());
@@ -951,7 +951,6 @@ void RedClient::handle_init(RedPeer::InMessage* message)
     _connection_id = init->session_id;
     set_mm_time(init->multi_media_time);
     calc_pixmap_cach_and_glz_window_size(init->display_channels_hint, init->ram_hint);
-    _glz_window.set_pixels_capacity(_glz_window_size);
     set_mouse_mode(init->supported_mouse_modes, init->current_mouse_mode);
     _agent_tokens = init->agent_tokens;
     _agent_connected = !!init->agent_connected;


More information about the Spice-commits mailing list