On Mon, 15 Aug 2011, Barry Song wrote: > >> @@ -580,7 +581,9 @@ static int fsg_lun_open(struct fsg_lun *curlun, const char *filename) > >> rc = (int) size; > >> goto out; > >> } > >> - num_sectors = size >> 9; /* File size in 512-byte blocks */ > >> + curlun->l_blkbits = blksize_bits( > >> + bdev_logical_block_size(inode->i_bdev)); > > > > Will this work if the backing file is not a block device? What if it's > > just a regular file? > > if it is just a regular file not a block device node, it has no bound > block device, that means i_bdev is null. > i prefer we make it have default logic block size 512-bytes. > if (inode->i_bdev) > curlun->l_blkbits = blksize_bits(bdev_logical_block_size(inode->i_bdev)); > else > curlun->l_blkbits = 9; Yes, that's better. > > As long as you're doing this, you might as well store (1 << l_blkbits) > > in curlun too, instead of recomputing it all the time. > > only in the read/write loop we re-calculate it, > nread -= nread & ((1 << curlun->l_blkbits) - 1); > nwritten -= (nwritten & ((1 << > curlun->l_blkbits) - 1)); > we might add a l_blksize field and replace the re-calculation by > nread -= nread & (curlun->l_blksize - 1); > nwritten -= (nwritten & > (curlun->l_blksize - 1)); That would be good. However I suggest you drop the "l_" prefix in the variable names. "curlun->blkbits" and "curlun->blksize" are perfectly understandable, and everyone will know the names refer to the logical block size. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html