Re: [Qemu-block] [Qemu-devel] [PATCH 2/2] sheepdog: remove huge BSS object

[Philippe Mathieu-Daudé]

On 05/22/2018 10:43 PM, Fam Zheng wrote:
> On Tue, 05/22 22:10, Paolo Bonzini wrote:
>> block/sheepdog.o has a 4M static variable that is 90% of QEMU's whole .bss
>> section.  Replace it with a heap-allocated block, and make it smaller too
>> since only the inode header is actually being used.
>>
>> bss size goes down from 4464280 to 269976.
>>
>> Signed-off-by: Paolo Bonzini <address@hidden>
>> ---
>>  block/sheepdog.c | 21 +++++++++++----------
>>  1 file changed, 11 insertions(+), 10 deletions(-)
>>
>> diff --git a/block/sheepdog.c b/block/sheepdog.c
>> index 23cf5a8430..2068166e3e 100644
>> --- a/block/sheepdog.c
>> +++ b/block/sheepdog.c
>> @@ -2940,13 +2940,14 @@ static int sd_snapshot_list(BlockDriverState *bs, 
>> QEMUSnapshotInfo **psn_tab)
>>      QEMUSnapshotInfo *sn_tab = NULL;
>>      unsigned wlen, rlen;
>>      int found = 0;
>> -    static SheepdogInode inode;
>> +    SheepdogInode *inode;
>>      unsigned long *vdi_inuse;
>>      unsigned int start_nr;
>>      uint64_t hval;
>>      uint32_t vid;
>>  
>>      vdi_inuse = g_malloc(max);
>> +    inode = g_malloc(SD_INODE_HEADER_SIZE);
> 
> Should you g_free() it before returning?

Oops good catch

> 
> 
>>  
>>      fd = connect_to_sdog(s, &local_err);
>>      if (fd < 0) {
>> @@ -2989,7 +2990,7 @@ static int sd_snapshot_list(BlockDriverState *bs, 
>> QEMUSnapshotInfo **psn_tab)
>>          }
>>  
>>          /* we don't need to read entire object */
>> -        ret = read_object(fd, s->bs, (char *)&inode,
>> +        ret = read_object(fd, s->bs, (char *)inode,
>>                            vid_to_vdi_oid(vid),
>>                            0, SD_INODE_HEADER_SIZE, 0,
>>                            s->cache_flags);
>> @@ -2998,17 +2999,17 @@ static int sd_snapshot_list(BlockDriverState *bs, 
>> QEMUSnapshotInfo **psn_tab)
>>              continue;
>>          }
>>  
>> -        if (!strcmp(inode.name, s->name) && is_snapshot(&inode)) {
>> -            sn_tab[found].date_sec = inode.snap_ctime >> 32;
>> -            sn_tab[found].date_nsec = inode.snap_ctime & 0xffffffff;
>> -            sn_tab[found].vm_state_size = inode.vm_state_size;
>> -            sn_tab[found].vm_clock_nsec = inode.vm_clock_nsec;
>> +        if (!strcmp(inode->name, s->name) && is_snapshot(inode)) {
>> +            sn_tab[found].date_sec = inode->snap_ctime >> 32;
>> +            sn_tab[found].date_nsec = inode->snap_ctime & 0xffffffff;
>> +            sn_tab[found].vm_state_size = inode->vm_state_size;
>> +            sn_tab[found].vm_clock_nsec = inode->vm_clock_nsec;
>>  
>>              snprintf(sn_tab[found].id_str, sizeof(sn_tab[found].id_str),
>> -                     "%" PRIu32, inode.snap_id);
>> +                     "%" PRIu32, inode->snap_id);
>>              pstrcpy(sn_tab[found].name,
>> -                    MIN(sizeof(sn_tab[found].name), sizeof(inode.tag)),
>> -                    inode.tag);
>> +                    MIN(sizeof(sn_tab[found].name), sizeof(inode->tag)),
>> +                    inode->tag);
>>              found++;
>>          }
>>      }
>> -- 
>> 2.17.0
>>
>>
>