On Fri, Feb 07, 2020 at 03:26:47PM -0500, Vivek Goyal wrote:
> Currently pmem_do_write() is written with assumption that all I/O is
> sector aligned. Soon I want to use this function in zero_page_range()
> where range passed in does not have to be sector aligned.
> 
> Modify this function to be able to deal with an arbitrary range. Which
> is specified by pmem_off and len.
> 
> Signed-off-by: Vivek Goyal <[email protected]>
> ---
>  drivers/nvdimm/pmem.c | 30 ++++++++++++++++++++++--------
>  1 file changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> index 9ad07cb8c9fc..281fe04d25fd 100644
> --- a/drivers/nvdimm/pmem.c
> +++ b/drivers/nvdimm/pmem.c
> @@ -154,15 +154,23 @@ static blk_status_t pmem_do_read(struct pmem_device 
> *pmem,
>  
>  static blk_status_t pmem_do_write(struct pmem_device *pmem,
>                       struct page *page, unsigned int page_off,
> -                     sector_t sector, unsigned int len)
> +                     u64 pmem_off, unsigned int len)
>  {
>       blk_status_t rc = BLK_STS_OK;
>       bool bad_pmem = false;
> -     phys_addr_t pmem_off = sector * 512 + pmem->data_offset;
> -     void *pmem_addr = pmem->virt_addr + pmem_off;
> -
> -     if (unlikely(is_bad_pmem(&pmem->bb, sector, len)))
> -             bad_pmem = true;
> +     phys_addr_t pmem_real_off = pmem_off + pmem->data_offset;
> +     void *pmem_addr = pmem->virt_addr + pmem_real_off;
> +     sector_t sector_start, sector_end;
> +     unsigned nr_sectors;
> +
> +     sector_start = DIV_ROUND_UP(pmem_off, SECTOR_SIZE);
> +     sector_end = (pmem_off + len) >> SECTOR_SHIFT;
> +     if (sector_end > sector_start) {
> +             nr_sectors = sector_end - sector_start;
> +             if (unlikely(is_bad_pmem(&pmem->bb, sector_start,
> +                                      nr_sectors << SECTOR_SHIFT)))
> +                     bad_pmem = true;

I don't think an unlikely annotation makes much sense for assigning
a boolean value to a flag variable.

> +             /*
> +              * Pass sector aligned offset and length. That seems
> +              * to work as of now. Other finer grained alignment
> +              * cases can be addressed later if need be.
> +              */

This comment seems pretty scary.  What other cases can you think of?


--
dm-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/dm-devel

Reply via email to