diff for duplicates of <1490657921.7897.20.camel@sandisk.com> diff --git a/a/1.txt b/N1/1.txt index 229efe5..09d79aa 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,20 +1,18 @@ On Mon, 2017-03-20 at 16:43 -0400, Christoph Hellwig wrote: -> @@ -698,13 +698,19 @@ static void sd_config_discard(struct scsi_disk *sdk= -p, unsigned int mode) +> @@ -698,13 +698,19 @@ static void sd_config_discard(struct scsi_disk *sdkp, unsigned int mode) > break; -> =20 +> > case SD_LBP_ATA_TRIM: -> - max_blocks =3D 65535 * (512 / sizeof(__le64)); -> + max_ranges =3D 512 / sizeof(__le64); -> + max_range_size =3D USHRT_MAX; -> + max_blocks =3D max_ranges * max_range_size; +> - max_blocks = 65535 * (512 / sizeof(__le64)); +> + max_ranges = 512 / sizeof(__le64); +> + max_range_size = USHRT_MAX; +> + max_blocks = max_ranges * max_range_size; > if (sdkp->device->ata_trim_zeroes_data) -> q->limits.discard_zeroes_data =3D 1; +> q->limits.discard_zeroes_data = 1; > break; > } -> =20 -> blk_queue_max_discard_sectors(q, max_blocks * (logical_block_size >> 9)= -); +> +> blk_queue_max_discard_sectors(q, max_blocks * (logical_block_size >> 9)); > + if (max_ranges) > + blk_queue_max_discard_segments(q, max_ranges); > + if (max_range_size) @@ -24,42 +22,39 @@ p, unsigned int mode) Hello Christoph, -Does blk_queue_max_discard_segment_size() expect a second argument that is = -a -number of bytes? Is max_range_size a number of logical blocks that each hav= -e +Does blk_queue_max_discard_segment_size() expect a second argument that is a +number of bytes? Is max_range_size a number of logical blocks that each have a size 1 << sector_shift? -> @@ -826,14 +832,21 @@ static int sd_setup_ata_trim_cmnd(struct scsi_cmnd = -*cmd) -> cmd->cmnd[8] =3D data_len; -> =20 -> buf =3D page_address(rq->special_vec.bv_page); -> - for (i =3D 0; i < (data_len >> 3); i++) { -> - u64 n =3D min(nr_sectors, 0xffffu); +> @@ -826,14 +832,21 @@ static int sd_setup_ata_trim_cmnd(struct scsi_cmnd *cmd) +> cmd->cmnd[8] = data_len; +> +> buf = page_address(rq->special_vec.bv_page); +> - for (i = 0; i < (data_len >> 3); i++) { +> - u64 n = min(nr_sectors, 0xffffu); > + __rq_for_each_bio(bio, rq) { -> + u64 sector =3D bio->bi_iter.bi_sector >> (sector_shift - 9); -> + u32 nr_sectors =3D bio->bi_iter.bi_size >> sector_shift; -> =20 -> - buf[i] =3D cpu_to_le64(sector | (n << 48)); -> - if (nr_sectors <=3D 0xffff) +> + u64 sector = bio->bi_iter.bi_sector >> (sector_shift - 9); +> + u32 nr_sectors = bio->bi_iter.bi_size >> sector_shift; +> +> - buf[i] = cpu_to_le64(sector | (n << 48)); +> - if (nr_sectors <= 0xffff) > - break; -> - sector +=3D 0xffff; -> - nr_sectors -=3D 0xffff; +> - sector += 0xffff; +> - nr_sectors -= 0xffff; > + do { -> + u64 n =3D min(nr_sectors, 0xffffu); +> + u64 n = min(nr_sectors, 0xffffu); > + -> + buf[i] =3D cpu_to_le64(sector | (n << 48)); -> + if (nr_sectors <=3D 0xffff) +> + buf[i] = cpu_to_le64(sector | (n << 48)); +> + if (nr_sectors <= 0xffff) > + break; -> + sector +=3D 0xffff; -> + nr_sectors -=3D 0xffff; +> + sector += 0xffff; +> + nr_sectors -= 0xffff; > + i++; > + -> + } while (!WARN_ON_ONCE(i >=3D data_len >> 3)); +> + } while (!WARN_ON_ONCE(i >= data_len >> 3)); > } -> =20 -> cmd->allowed =3D SD_MAX_RETRIES; +> +> cmd->allowed = SD_MAX_RETRIES; It's unfortunate that the loop-end test occurs twice (i < data_len >> 3). Please consider using put_unaligned_le64() instead of cpu_to_le64() such @@ -68,12 +63,12 @@ that change and by introducing e.g. the following: void *end; [ ... ] -end =3D (void *)buf + data_len; +end = (void *)buf + data_len; -the loop variable 'i' can be eliminated. If buf[i++] =3D ... would be -changed into *buf++ =3D ... then that would allow to change the two +the loop variable 'i' can be eliminated. If buf[i++] = ... would be +changed into *buf++ = ... then that would allow to change the two occurrences of (i < data_len >> 3) into (buf < end). Thanks, -Bart.= +Bart. diff --git a/a/content_digest b/N1/content_digest index 70ad54a..14c4d47 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -13,22 +13,20 @@ "\00:1\0" "b\0" "On Mon, 2017-03-20 at 16:43 -0400, Christoph Hellwig wrote:\n" - "> @@ -698,13 +698,19 @@ static void sd_config_discard(struct scsi_disk *sdk=\n" - "p, unsigned int mode)\n" + "> @@ -698,13 +698,19 @@ static void sd_config_discard(struct scsi_disk *sdkp, unsigned int mode)\n" "> \t\tbreak;\n" - "> =20\n" + "> \n" "> \tcase SD_LBP_ATA_TRIM:\n" - "> -\t\tmax_blocks =3D 65535 * (512 / sizeof(__le64));\n" - "> +\t\tmax_ranges =3D 512 / sizeof(__le64);\n" - "> +\t\tmax_range_size =3D USHRT_MAX;\n" - "> +\t\tmax_blocks =3D max_ranges * max_range_size;\n" + "> -\t\tmax_blocks = 65535 * (512 / sizeof(__le64));\n" + "> +\t\tmax_ranges = 512 / sizeof(__le64);\n" + "> +\t\tmax_range_size = USHRT_MAX;\n" + "> +\t\tmax_blocks = max_ranges * max_range_size;\n" "> \t\tif (sdkp->device->ata_trim_zeroes_data)\n" - "> \t\t\tq->limits.discard_zeroes_data =3D 1;\n" + "> \t\t\tq->limits.discard_zeroes_data = 1;\n" "> \t\tbreak;\n" "> \t}\n" - "> =20\n" - "> \tblk_queue_max_discard_sectors(q, max_blocks * (logical_block_size >> 9)=\n" - ");\n" + "> \n" + "> \tblk_queue_max_discard_sectors(q, max_blocks * (logical_block_size >> 9));\n" "> +\tif (max_ranges)\n" "> +\t\tblk_queue_max_discard_segments(q, max_ranges);\n" "> +\tif (max_range_size)\n" @@ -38,42 +36,39 @@ "\n" "Hello Christoph,\n" "\n" - "Does blk_queue_max_discard_segment_size() expect a second argument that is =\n" - "a\n" - "number of bytes? Is max_range_size a number of logical blocks that each hav=\n" - "e\n" + "Does blk_queue_max_discard_segment_size() expect a second argument that is a\n" + "number of bytes? Is max_range_size a number of logical blocks that each have\n" "a size 1 << sector_shift?\n" "\n" - "> @@ -826,14 +832,21 @@ static int sd_setup_ata_trim_cmnd(struct scsi_cmnd =\n" - "*cmd)\n" - "> \tcmd->cmnd[8] =3D data_len;\n" - "> =20\n" - "> \tbuf =3D page_address(rq->special_vec.bv_page);\n" - "> -\tfor (i =3D 0; i < (data_len >> 3); i++) {\n" - "> -\t\tu64 n =3D min(nr_sectors, 0xffffu);\n" + "> @@ -826,14 +832,21 @@ static int sd_setup_ata_trim_cmnd(struct scsi_cmnd *cmd)\n" + "> \tcmd->cmnd[8] = data_len;\n" + "> \n" + "> \tbuf = page_address(rq->special_vec.bv_page);\n" + "> -\tfor (i = 0; i < (data_len >> 3); i++) {\n" + "> -\t\tu64 n = min(nr_sectors, 0xffffu);\n" "> +\t__rq_for_each_bio(bio, rq) {\n" - "> +\t\tu64 sector =3D bio->bi_iter.bi_sector >> (sector_shift - 9);\n" - "> +\t\tu32 nr_sectors =3D bio->bi_iter.bi_size >> sector_shift;\n" - "> =20\n" - "> -\t\tbuf[i] =3D cpu_to_le64(sector | (n << 48));\n" - "> -\t\tif (nr_sectors <=3D 0xffff)\n" + "> +\t\tu64 sector = bio->bi_iter.bi_sector >> (sector_shift - 9);\n" + "> +\t\tu32 nr_sectors = bio->bi_iter.bi_size >> sector_shift;\n" + "> \n" + "> -\t\tbuf[i] = cpu_to_le64(sector | (n << 48));\n" + "> -\t\tif (nr_sectors <= 0xffff)\n" "> -\t\t\tbreak;\n" - "> -\t\tsector +=3D 0xffff;\n" - "> -\t\tnr_sectors -=3D 0xffff;\n" + "> -\t\tsector += 0xffff;\n" + "> -\t\tnr_sectors -= 0xffff;\n" "> +\t\tdo {\n" - "> +\t\t\tu64 n =3D min(nr_sectors, 0xffffu);\n" + "> +\t\t\tu64 n = min(nr_sectors, 0xffffu);\n" "> +\n" - "> +\t\t\tbuf[i] =3D cpu_to_le64(sector | (n << 48));\n" - "> +\t\t\tif (nr_sectors <=3D 0xffff)\n" + "> +\t\t\tbuf[i] = cpu_to_le64(sector | (n << 48));\n" + "> +\t\t\tif (nr_sectors <= 0xffff)\n" "> +\t\t\t\tbreak;\n" - "> +\t\t\tsector +=3D 0xffff;\n" - "> +\t\t\tnr_sectors -=3D 0xffff;\n" + "> +\t\t\tsector += 0xffff;\n" + "> +\t\t\tnr_sectors -= 0xffff;\n" "> +\t\t\ti++;\n" "> +\n" - "> +\t\t} while (!WARN_ON_ONCE(i >=3D data_len >> 3));\n" + "> +\t\t} while (!WARN_ON_ONCE(i >= data_len >> 3));\n" "> \t}\n" - "> =20\n" - "> \tcmd->allowed =3D SD_MAX_RETRIES;\n" + "> \n" + "> \tcmd->allowed = SD_MAX_RETRIES;\n" "\n" "It's unfortunate that the loop-end test occurs twice (i < data_len >> 3).\n" "Please consider using put_unaligned_le64() instead of cpu_to_le64() such\n" @@ -82,14 +77,14 @@ "\n" "void *end;\n" "[ ... ]\n" - "end =3D (void *)buf + data_len;\n" + "end = (void *)buf + data_len;\n" "\n" - "the loop variable 'i' can be eliminated. If buf[i++] =3D ... would be\n" - "changed into *buf++ =3D ... then that would allow to change the two\n" + "the loop variable 'i' can be eliminated. If buf[i++] = ... would be\n" + "changed into *buf++ = ... then that would allow to change the two\n" "occurrences of (i < data_len >> 3) into (buf < end).\n" "\n" "Thanks,\n" "\n" - Bart.= + Bart. -69146b5dbf01f1cf6c944dcdce2d98f0836cb7332a096242cc4b9f726a962dd3 +141decc3d0304e47ef794528ddd99c80fd3a172ad741408727f5bc3f0339c1eb
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.