Re: [PATCH 1/3] zram: fix operator precedence to get offset

Minchan Kim
2017-04-17T02:15:32Z

Hi Sergey,

On Mon, Apr 17, 2017 at 10:54:29AM +0900, Sergey Senozhatsky wrote:
> On (04/17/17 10:21), Sergey Senozhatsky wrote:
> > > However, it should be *fixed* to prevent confusion in future
>
> or may be something like below? can save us some cycles.
>
> remove this calculation
>
> - offset = sector & (SECTORS_PER_PAGE - 1) << SECTOR_SHIFT;
>
>
> and pass 0 to zram_bvec_rw()
>
> - err = zram_bvec_rw(zram, &bv, index, offset, is_write);
> + err = zram_bvec_rw(zram, &bv, index, 0, is_write);

That was one I wrote but have thought it more.

Because I suspect fs can submit page-size IO in non-aligned PAGE_SIZE
sector? For example, it can submit PAGE_SIZE read request from 9 sector.
Is it possible? I don't know.

As well, FS can format zram from sector 1, not sector 0? IOW, can't it
use starting sector as non-page algined sector?
We can do it via fdisk?

Anyway, If one of scenario I mentioned is possible, zram_rw_page will
be broken.

If it's hard to check all of scenario in this moment, it would be
better to not remove it and then add WARN_ON(offset) in there.

While I am writing this, I found this.

/**
* bdev_read_page() - Start reading a page from a block device
* @bdev: The device to read the page from
* @sector: The offset on the device to read the page to (need not be aligned)
* @page: The page to read
*

Hmm,, need investigation but no time.