From 04eee25b1d754a837360504b7af426d1f86ffeb7 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Thu, 18 Aug 2011 14:29:00 -0400 Subject: [PATCH] USB: gadget: storage: remove alignment assumption 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 Acked-by: Michal Nazarewicz Signed-off-by: Felipe Balbi --- drivers/usb/gadget/f_mass_storage.c | 63 ++++++++++++---------------------- drivers/usb/gadget/file_storage.c | 67 +++++++++++++++---------------------- 2 files changed, 49 insertions(+), 81 deletions(-) diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c index 1cdb1fa1b15..4306a833948 100644 --- a/drivers/usb/gadget/f_mass_storage.c +++ b/drivers/usb/gadget/f_mass_storage.c @@ -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; /* @@ -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; @@ -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; @@ -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; @@ -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; @@ -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; @@ -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; @@ -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, @@ -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; @@ -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, @@ -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; diff --git a/drivers/usb/gadget/file_storage.c b/drivers/usb/gadget/file_storage.c index 59d97750cec..c6f96a2b311 100644 --- a/drivers/usb/gadget/file_storage.c +++ b/drivers/usb/gadget/file_storage.c @@ -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 @@ -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; @@ -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; @@ -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; @@ -1312,23 +1308,13 @@ 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; @@ -1336,14 +1322,6 @@ static int do_write(struct fsg_dev *fsg) 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; @@ -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; @@ -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, @@ -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; @@ -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); @@ -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; -- 2.11.4.GIT