[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Rewrite and fix grub_bufio_read()
From: |
Andrei Borzenkov |
Subject: |
Re: [PATCH] Rewrite and fix grub_bufio_read() |
Date: |
Fri, 29 Apr 2016 06:50:46 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 |
28.04.2016 22:52, Stefan Fritsch пишет:
> On Thu, 28 Apr 2016, Andrei Borzenkov wrote:
>
>> 28.04.2016 14:11, Stefan Fritsch пишет:
>>> We had problems downloading large (10-100 MB) gziped files via http
>>> with grub. After some debugging, I think I have found the reason in
>>> grub_bufio_read, which leads to wrong data being passed on. This patch
>>> fixes the issues for me. I did my tests with a somewhat older
>>> snapshot, but the bufio.c file is the same.
>>>
>>
>> Please send minimal patch that fixes bug in bufio so we can actually see
>> where the bug is and apply bug fix for 2.02. It is impossible to deduce
>> from your complete rewrite and this patch is too intrusive for 2.02
>> irrespectively of considerations below.
>>
>>> Cheers,
>>> Stefan
>>>
>>> The grub_bufio_read() had some issues:
>>>
>>> * in the calculation of next_buf, it assumed that bufio->block_size is a
>>> power of 2, which is not always the case (see code in grub_bufio_open()).
>>>
>>
>> All current callers set it to power of 2, although you are right that it
>> should verify it.
>
> No:
>
> if ((size < 0) || ((unsigned) size > io->size))
> size = ((io->size > GRUB_BUFIO_MAX_SIZE) ? GRUB_BUFIO_MAX_SIZE :
> io->size);
>
> ...
>
> bufio->block_size = size;
>
> io->size is the file size. So at least for files < 32K (which is the size
> that bufio is called with by the net layer), block_size is not a power of
> 2 but the size of the file. Then next_buf typically gets a value from 0
> to 8.
>
Ah, OK. Not sure why we need this check in the first place. Buffer size
is unrelated to file size, so this looks more like micro-optimization of
buffer size.
Although this means that we always have the whole file in buffer anyway
and so never hit power of 2 issue.
- [PATCH] Rewrite and fix grub_bufio_read(), Stefan Fritsch, 2016/04/28
- Re: [PATCH] Rewrite and fix grub_bufio_read(), Andrei Borzenkov, 2016/04/28
- Re: [PATCH] Rewrite and fix grub_bufio_read(), Stefan Fritsch, 2016/04/28
- gzio/http problem (was: [PATCH] Rewrite and fix grub_bufio_read()), Stefan Fritsch, 2016/04/28
- Re: gzio/http problem (was: [PATCH] Rewrite and fix grub_bufio_read()), Vladimir 'phcoder' Serbinenko, 2016/04/28
- Re: gzio/http problem, Andrei Borzenkov, 2016/04/29
- Re: gzio/http problem, Stefan Fritsch, 2016/04/29
- Re: gzio/http problem, Andrei Borzenkov, 2016/04/30
- Re: [PATCH] Rewrite and fix grub_bufio_read(),
Andrei Borzenkov <=