On Mon, 2008-01-14 at 20:27 +0100, Hans de Goede wrote: > James Bottomley wrote: > >>> Plus what is the rq->nr_sectors > sdp->sector_size / > >>> 512 test supposed to be doing? that being true is supposed to be a > >>> guarantee of the block layer (and if something goes wrong there's a > >>> check for this lower down). > >>> > >> It first is was just: > >> rq->nr_sectors > 1 > >> > >> Then I changed it to also do the right thing for 1024 and larger sector disks. > >> The whole purpose is to not read the last sector in a larger then one sector > >> read, so the amount of sectors gets reduced by one if (block + rq->nr_sectors > >> == get_capacity(disk)) but we do want still want to be able to read the last > >> sector by itself, so we must not reduce the no sectors to be read by one if it > >> is already one. > > > > Yes, I know that. What I mean is the block subsystem sends reads and > > writes down in increments of the sector size. Checking if the current > > number of pending sectors is greater than the current block size is > > checking that guarantee. The current code already has a check in it to > > see if this guarantee is observed ... you don't need to check it again. > > > > I'm not checking for the guarantee, I'm checking that the read > 1 > realsize_sector, before decrementing the number of _512_bytes_sectors by > realsize_sector, this check must be there, as after reading all but the last > sector, another request will be send with 1 realsize_sector size, for which > (block + rq->nr_sectors) == get_capacity(disk) will still hold true, in this > case this_count must not be lowered, otherwise this_count will become 0 in this > case and the last sector will never get read. > > I hope that clarifies why that code is there, if not can you tell how you would > want the code to look by just giving the full if statement as it should be, I > think we are somehow misunderstanding each other. Oh, right; it's > rather than >= ... sorry, yes that's fine. James - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
