we want to skip the logical block which is partially covered by
the discard bio, so check the remaining size and subtract it if
there is a need to goto the next logical block.
This patch corrects the offset usage in zram_bio_discard.
Signed-off-by: Weijie Yang <[email protected]>
---
drivers/block/zram/zram_drv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 9849b52..48eccb3 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -572,10 +572,10 @@ static void zram_bio_discard(struct zram *zram, u32 index,
* skipping this logical block is appropriate here.
*/
if (offset) {
- if (n < offset)
+ if (n <= (PAGE_SIZE - offset))
return;
- n -= offset;
+ n -= (PAGE_SIZE - offset);
index++;
}
--
1.7.10.4
On (04/22/14 11:14), Weijie Yang wrote:
>
> we want to skip the logical block which is partially covered by
> the discard bio, so check the remaining size and subtract it if
> there is a need to goto the next logical block.
>
looks sane.
-ss
> This patch corrects the offset usage in zram_bio_discard.
>
> Signed-off-by: Weijie Yang <[email protected]>
> ---
> drivers/block/zram/zram_drv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index 9849b52..48eccb3 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -572,10 +572,10 @@ static void zram_bio_discard(struct zram *zram, u32 index,
> * skipping this logical block is appropriate here.
> */
> if (offset) {
> - if (n < offset)
> + if (n <= (PAGE_SIZE - offset))
> return;
>
> - n -= offset;
> + n -= (PAGE_SIZE - offset);
> index++;
> }
>
> --
> 1.7.10.4
>
>
On Tue, 22 Apr 2014 11:14:02 +0800 Weijie Yang <[email protected]> wrote:
> we want to skip the logical block which is partially covered by
> the discard bio, so check the remaining size and subtract it if
> there is a need to goto the next logical block.
>
> This patch corrects the offset usage in zram_bio_discard.
>
What were the end-user visible effects of the bug?
Please always include this information when fixing something so that
others can work out which kernel(s) need patching.
On Wed, Apr 23, 2014 at 3:55 AM, Andrew Morton
<[email protected]> wrote:
> On Tue, 22 Apr 2014 11:14:02 +0800 Weijie Yang <[email protected]> wrote:
>
>> we want to skip the logical block which is partially covered by
>> the discard bio, so check the remaining size and subtract it if
>> there is a need to goto the next logical block.
>>
>> This patch corrects the offset usage in zram_bio_discard.
>>
>
> What were the end-user visible effects of the bug?
>
> Please always include this information when fixing something so that
> others can work out which kernel(s) need patching.
>
Thanks for your advise, I will resend this patch and add the end-user
visible effect information.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
On Wed, Apr 23, 2014 at 10:32:30AM +0800, Weijie Yang wrote:
> On Wed, Apr 23, 2014 at 3:55 AM, Andrew Morton
> <[email protected]> wrote:
> > On Tue, 22 Apr 2014 11:14:02 +0800 Weijie Yang <[email protected]> wrote:
> >
> >> we want to skip the logical block which is partially covered by
> >> the discard bio, so check the remaining size and subtract it if
> >> there is a need to goto the next logical block.
> >>
> >> This patch corrects the offset usage in zram_bio_discard.
> >>
> >
> > What were the end-user visible effects of the bug?
> >
> > Please always include this information when fixing something so that
> > others can work out which kernel(s) need patching.
> >
>
> Thanks for your advise, I will resend this patch and add the end-user
> visible effect information.
Thanks for fixing it.
As far as I understand, there is no end-user visible effect, because
request size is alway PAGE_SIZE aligned and if n < PAGE_SIZE,
no real operation happens. Am I missing?
Anyway,
Acked-by: Joonsoo Kim <[email protected]>
Thanks.
On Wed, Apr 23, 2014 at 11:08 AM, Joonsoo Kim <[email protected]> wrote:
> On Wed, Apr 23, 2014 at 10:32:30AM +0800, Weijie Yang wrote:
>> On Wed, Apr 23, 2014 at 3:55 AM, Andrew Morton
>> <[email protected]> wrote:
>> > On Tue, 22 Apr 2014 11:14:02 +0800 Weijie Yang <[email protected]> wrote:
>> >
>> >> we want to skip the logical block which is partially covered by
>> >> the discard bio, so check the remaining size and subtract it if
>> >> there is a need to goto the next logical block.
>> >>
>> >> This patch corrects the offset usage in zram_bio_discard.
>> >>
>> >
>> > What were the end-user visible effects of the bug?
>> >
>> > Please always include this information when fixing something so that
>> > others can work out which kernel(s) need patching.
>> >
>>
>> Thanks for your advise, I will resend this patch and add the end-user
>> visible effect information.
>
> Thanks for fixing it.
>
> As far as I understand, there is no end-user visible effect, because
> request size is alway PAGE_SIZE aligned and if n < PAGE_SIZE,
> no real operation happens. Am I missing?
The zram only limit ZRAM_LOGICAL_BLOCK_SIZE(4K) aligned,
not PAGE_SIZE aligned.
Consider the following scenario:
on some architecture or config, PAGE_SIZE is 64K for example
filesystem is set up on zram disk without PAGE_SIZE aligned.
a discard bio leads to a offset = 4K, size=72K
normally, it should not really discard any physical block as it partially
cover two physical blocks.
However, with the current offset usage, it will discard the second
physical block and free its memory, which will cause filesystem breakdown.
regards,
> Anyway,
>
> Acked-by: Joonsoo Kim <[email protected]>
>
> Thanks.
On Wed, Apr 23, 2014 at 11:52:08AM +0800, Weijie Yang wrote:
> On Wed, Apr 23, 2014 at 11:08 AM, Joonsoo Kim <[email protected]> wrote:
> > On Wed, Apr 23, 2014 at 10:32:30AM +0800, Weijie Yang wrote:
> >> On Wed, Apr 23, 2014 at 3:55 AM, Andrew Morton
> >> <[email protected]> wrote:
> >> > On Tue, 22 Apr 2014 11:14:02 +0800 Weijie Yang <[email protected]> wrote:
> >> >
> >> >> we want to skip the logical block which is partially covered by
> >> >> the discard bio, so check the remaining size and subtract it if
> >> >> there is a need to goto the next logical block.
> >> >>
> >> >> This patch corrects the offset usage in zram_bio_discard.
> >> >>
> >> >
> >> > What were the end-user visible effects of the bug?
> >> >
> >> > Please always include this information when fixing something so that
> >> > others can work out which kernel(s) need patching.
> >> >
> >>
> >> Thanks for your advise, I will resend this patch and add the end-user
> >> visible effect information.
> >
> > Thanks for fixing it.
> >
> > As far as I understand, there is no end-user visible effect, because
> > request size is alway PAGE_SIZE aligned and if n < PAGE_SIZE,
> > no real operation happens. Am I missing?
>
> The zram only limit ZRAM_LOGICAL_BLOCK_SIZE(4K) aligned,
> not PAGE_SIZE aligned.
>
> Consider the following scenario:
> on some architecture or config, PAGE_SIZE is 64K for example
> filesystem is set up on zram disk without PAGE_SIZE aligned.
> a discard bio leads to a offset = 4K, size=72K
> normally, it should not really discard any physical block as it partially
> cover two physical blocks.
> However, with the current offset usage, it will discard the second
> physical block and free its memory, which will cause filesystem breakdown.
>
I misunderstood what discard_granularity means. I thought that if I set
discard_granularity=PAGE_SIZE, the size of discard request is aligned to
PAGE_SIZE. But, by looking at the code, I notice that isn't true. So now, I
understand your patch compeletely. Thanks again.
Thanks.