Messages in this thread | | | Date | Thu, 16 Jun 2022 09:37:15 +0800 | Subject | Re: [PATCH v2] scsi: support packing multi-segment in UNMAP command | From | Chao Yu <> |
| |
On 2022/6/15 21:35, Christoph Hellwig wrote: > On Wed, Jun 15, 2022 at 09:28:46PM +0800, Chao Yu wrote: >> As SPEC describes that it can support unmapping one or more LBA range > > s/SPEC/<insert SPC reference here>/ > >> { >> struct scsi_device *sdp = cmd->device; >> struct request *rq = scsi_cmd_to_rq(cmd); >> + struct bio *bio; >> struct scsi_disk *sdkp = scsi_disk(rq->q->disk); >> + unsigned short segments = blk_rq_nr_discard_segments(rq); >> + unsigned int data_len = 8 + 16 * segments, i = 0; >> char *buf; > > Nit: for readability I'd move the bio declaration just above the buf > line. > >> + >> + __rq_for_each_bio(bio, rq) { >> + u64 lba = sectors_to_logical(sdp, bio->bi_iter.bi_sector); >> + u32 nr_blocks = sectors_to_logical(sdp, bio_sectors(bio)); >> + >> + put_unaligned_be64(lba, &buf[8 + 16 * i]); >> + put_unaligned_be32(nr_blocks, &buf[8 + 16 * i + 8]); > > Can we have a local variable here? > > unsigned int data_offset = 8; > > .... >
Thanks for the review, I've updated the patch.
Thanks,
> put_unaligned_be64(lba, &buf[data_offset]); > put_unaligned_be64(lba, &buf[data_offset + 8]); > data_offset += 16; > } >
| |