Re: [PATCH] gadget: mass_storage: adapt logic block size to bound block devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux