Skip to content

Commit

Permalink
USB: gadget: storage: remove alignment assumption
Browse files Browse the repository at this point in the history 
This patch (as1481) fixes a problem affecting g_file_storage and
g_mass_storage when running at SuperSpeed.  The two drivers currently
assume that the bulk-out maxpacket size can evenly divide the SCSI
block size, which is 512 bytes.  But SuperSpeed bulk endpoints have a
maxpacket size of 1024, so the assumption is no longer true.

This patch removes that assumption from the drivers, by getting rid of
a small optimization (they try to align VFS reads and writes on page
cache boundaries).  If a command's starting logical block address is
512 bytes below the end of a page, it's not okay to issue a USB
command for just those 512 bytes when the maxpacket size is 1024 -- it
would result in either babble (for an OUT transfer) or a short packet
(for an IN transfer).

Also, for backward compatibility, the test for writes extending beyond
the end of the backing storage has to be changed.  If the host tries
to do this, we should accept the data that fits in the backing storage
and ignore the rest.  Because the storage's end may not align with a
USB packet boundary, this means we may have to accept a USB OUT
transfer that extends beyond the end of the storage and then write out
only the part of the data that fits.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Acked-by: Michal Nazarewicz <mina86@mina86.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
  • Loading branch information
Alan Stern authored and Felipe Balbi committed Sep 9, 2011
1 parent 3f565a3 commit 04eee25
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 81 deletions.
63 changes: 22 additions & 41 deletions drivers/usb/gadget/f_mass_storage.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,6 @@ static int do_read(struct fsg_common *common)
u32 amount_left;
loff_t file_offset, file_offset_tmp;
unsigned int amount;
unsigned int partial_page;
ssize_t nread;

/*
Expand Down Expand Up @@ -783,18 +782,10 @@ static int do_read(struct fsg_common *common)
* Try to read the remaining amount.
* But don't read more than the buffer size.
* And don't try to read past the end of the file.
* Finally, if we're not at a page boundary, don't read past
* the next page.
* If this means reading 0 then we were asked to read past
* the end of file.
*/
amount = min(amount_left, FSG_BUFLEN);
amount = min((loff_t)amount,
curlun->file_length - file_offset);
partial_page = file_offset & (PAGE_CACHE_SIZE - 1);
if (partial_page > 0)
amount = min(amount, (unsigned int)PAGE_CACHE_SIZE -
partial_page);

/* Wait for the next buffer to become available */
bh = common->next_buffhd_to_fill;
Expand Down Expand Up @@ -840,6 +831,12 @@ static int do_read(struct fsg_common *common)
file_offset += nread;
amount_left -= nread;
common->residue -= nread;

/*
* Except at the end of the transfer, nread will be
* equal to the buffer size, which is divisible by the
* bulk-in maxpacket size.
*/
bh->inreq->length = nread;
bh->state = BUF_STATE_FULL;

Expand Down Expand Up @@ -878,7 +875,6 @@ static int do_write(struct fsg_common *common)
u32 amount_left_to_req, amount_left_to_write;
loff_t usb_offset, file_offset, file_offset_tmp;
unsigned int amount;
unsigned int partial_page;
ssize_t nwritten;
int rc;

Expand Down Expand Up @@ -934,24 +930,13 @@ static int do_write(struct fsg_common *common)

/*
* Figure out how much we want to get:
* Try to get the remaining amount.
* But don't get more than the buffer size.
* And don't try to go past the end of the file.
* If we're not at a page boundary,
* don't go past the next page.
* If this means getting 0, then we were asked
* to write past the end of file.
* Finally, round down to a block boundary.
* Try to get the remaining amount,
* but not more than the buffer size.
*/
amount = min(amount_left_to_req, FSG_BUFLEN);
amount = min((loff_t)amount,
curlun->file_length - usb_offset);
partial_page = usb_offset & (PAGE_CACHE_SIZE - 1);
if (partial_page > 0)
amount = min(amount,
(unsigned int)PAGE_CACHE_SIZE - partial_page);

if (amount == 0) {

/* Beyond the end of the backing file? */
if (usb_offset >= curlun->file_length) {
get_some_more = 0;
curlun->sense_data =
SS_LOGICAL_BLOCK_ADDRESS_OUT_OF_RANGE;
Expand All @@ -960,16 +945,6 @@ static int do_write(struct fsg_common *common)
curlun->info_valid = 1;
continue;
}
amount = round_down(amount, curlun->blksize);
if (amount == 0) {

/*
* Why were we were asked to transfer a
* partial block?
*/
get_some_more = 0;
continue;
}

/* Get the next buffer */
usb_offset += amount;
Expand All @@ -979,8 +954,9 @@ static int do_write(struct fsg_common *common)
get_some_more = 0;

/*
* amount is always divisible by 512, hence by
* the bulk-out maxpacket size
* Except at the end of the transfer, amount will be
* equal to the buffer size, which is divisible by
* the bulk-out maxpacket size.
*/
bh->outreq->length = amount;
bh->bulk_out_intended_length = amount;
Expand Down Expand Up @@ -1019,6 +995,11 @@ static int do_write(struct fsg_common *common)
amount = curlun->file_length - file_offset;
}

/* Don't write a partial block */
amount = round_down(amount, curlun->blksize);
if (amount == 0)
goto empty_write;

/* Perform the write */
file_offset_tmp = file_offset;
nwritten = vfs_write(curlun->filp,
Expand Down Expand Up @@ -1051,6 +1032,7 @@ static int do_write(struct fsg_common *common)
break;
}

empty_write:
/* Did the host decide to stop early? */
if (bh->outreq->actual != bh->outreq->length) {
common->short_packet_received = 1;
Expand Down Expand Up @@ -1151,8 +1133,6 @@ static int do_verify(struct fsg_common *common)
* Try to read the remaining amount, but not more than
* the buffer size.
* And don't try to read past the end of the file.
* If this means reading 0 then we were asked to read
* past the end of file.
*/
amount = min(amount_left, FSG_BUFLEN);
amount = min((loff_t)amount,
Expand Down Expand Up @@ -1628,7 +1608,8 @@ static int throw_away_data(struct fsg_common *common)
amount = min(common->usb_amount_left, FSG_BUFLEN);

/*
* amount is always divisible by 512, hence by
* Except at the end of the transfer, amount will be
* equal to the buffer size, which is divisible by
* the bulk-out maxpacket size.
*/
bh->outreq->length = amount;
Expand Down
67 changes: 27 additions & 40 deletions drivers/usb/gadget/file_storage.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1135,7 +1135,6 @@ static int do_read(struct fsg_dev *fsg)
u32 amount_left;
loff_t file_offset, file_offset_tmp;
unsigned int amount;
unsigned int partial_page;
ssize_t nread;

/* Get the starting Logical Block Address and check that it's
Expand Down Expand Up @@ -1170,17 +1169,10 @@ static int do_read(struct fsg_dev *fsg)
* Try to read the remaining amount.
* But don't read more than the buffer size.
* And don't try to read past the end of the file.
* Finally, if we're not at a page boundary, don't read past
* the next page.
* If this means reading 0 then we were asked to read past
* the end of file. */
*/
amount = min((unsigned int) amount_left, mod_data.buflen);
amount = min((loff_t) amount,
curlun->file_length - file_offset);
partial_page = file_offset & (PAGE_CACHE_SIZE - 1);
if (partial_page > 0)
amount = min(amount, (unsigned int) PAGE_CACHE_SIZE -
partial_page);

/* Wait for the next buffer to become available */
bh = fsg->next_buffhd_to_fill;
Expand Down Expand Up @@ -1225,6 +1217,11 @@ static int do_read(struct fsg_dev *fsg)
file_offset += nread;
amount_left -= nread;
fsg->residue -= nread;

/* Except at the end of the transfer, nread will be
* equal to the buffer size, which is divisible by the
* bulk-in maxpacket size.
*/
bh->inreq->length = nread;
bh->state = BUF_STATE_FULL;

Expand Down Expand Up @@ -1261,7 +1258,6 @@ static int do_write(struct fsg_dev *fsg)
u32 amount_left_to_req, amount_left_to_write;
loff_t usb_offset, file_offset, file_offset_tmp;
unsigned int amount;
unsigned int partial_page;
ssize_t nwritten;
int rc;

Expand Down Expand Up @@ -1312,38 +1308,20 @@ static int do_write(struct fsg_dev *fsg)
if (bh->state == BUF_STATE_EMPTY && get_some_more) {

/* Figure out how much we want to get:
* Try to get the remaining amount.
* But don't get more than the buffer size.
* And don't try to go past the end of the file.
* If we're not at a page boundary,
* don't go past the next page.
* If this means getting 0, then we were asked
* to write past the end of file.
* Finally, round down to a block boundary. */
* Try to get the remaining amount,
* but not more than the buffer size.
*/
amount = min(amount_left_to_req, mod_data.buflen);
amount = min((loff_t) amount, curlun->file_length -
usb_offset);
partial_page = usb_offset & (PAGE_CACHE_SIZE - 1);
if (partial_page > 0)
amount = min(amount,
(unsigned int) PAGE_CACHE_SIZE - partial_page);

if (amount == 0) {

/* Beyond the end of the backing file? */
if (usb_offset >= curlun->file_length) {
get_some_more = 0;
curlun->sense_data =
SS_LOGICAL_BLOCK_ADDRESS_OUT_OF_RANGE;
curlun->sense_data_info = usb_offset >> curlun->blkbits;
curlun->info_valid = 1;
continue;
}
amount = round_down(amount, curlun->blksize);
if (amount == 0) {

/* Why were we were asked to transfer a
* partial block? */
get_some_more = 0;
continue;
}

/* Get the next buffer */
usb_offset += amount;
Expand All @@ -1352,8 +1330,10 @@ static int do_write(struct fsg_dev *fsg)
if (amount_left_to_req == 0)
get_some_more = 0;

/* amount is always divisible by 512, hence by
* the bulk-out maxpacket size */
/* Except at the end of the transfer, amount will be
* equal to the buffer size, which is divisible by
* the bulk-out maxpacket size.
*/
bh->outreq->length = bh->bulk_out_intended_length =
amount;
bh->outreq->short_not_ok = 1;
Expand Down Expand Up @@ -1389,6 +1369,11 @@ static int do_write(struct fsg_dev *fsg)
amount = curlun->file_length - file_offset;
}

/* Don't write a partial block */
amount = round_down(amount, curlun->blksize);
if (amount == 0)
goto empty_write;

/* Perform the write */
file_offset_tmp = file_offset;
nwritten = vfs_write(curlun->filp,
Expand Down Expand Up @@ -1421,6 +1406,7 @@ static int do_write(struct fsg_dev *fsg)
break;
}

empty_write:
/* Did the host decide to stop early? */
if (bh->outreq->actual != bh->outreq->length) {
fsg->short_packet_received = 1;
Expand Down Expand Up @@ -1517,8 +1503,7 @@ static int do_verify(struct fsg_dev *fsg)
* Try to read the remaining amount, but not more than
* the buffer size.
* And don't try to read past the end of the file.
* If this means reading 0 then we were asked to read
* past the end of file. */
*/
amount = min((unsigned int) amount_left, mod_data.buflen);
amount = min((loff_t) amount,
curlun->file_length - file_offset);
Expand Down Expand Up @@ -1981,8 +1966,10 @@ static int throw_away_data(struct fsg_dev *fsg)
amount = min(fsg->usb_amount_left,
(u32) mod_data.buflen);

/* amount is always divisible by 512, hence by
* the bulk-out maxpacket size */
/* Except at the end of the transfer, amount will be
* equal to the buffer size, which is divisible by
* the bulk-out maxpacket size.
*/
bh->outreq->length = bh->bulk_out_intended_length =
amount;
bh->outreq->short_not_ok = 1;
Expand Down

0 comments on commit 04eee25

Please sign in to comment.