| Message ID | 20180530194807.31657-6-longli@linuxonhyperv.com |
|---|---|
| State | New |
| Headers | show |
| Series | CIFS: Add direct I/O support | expand |
On 5/30/2018 3:47 PM, Long Li wrote: > From: Long Li <longli@microsoft.com> > > It's possible that the page offset is non-zero in the pages in a request, > change the function to calculate the correct data buffer length. > > Signed-off-by: Long Li <longli@microsoft.com> > --- > fs/cifs/transport.c | 20 +++++++++++++++++--- > 1 file changed, 17 insertions(+), 3 deletions(-) > > diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c > index 927226a..d6b5523 100644 > --- a/fs/cifs/transport.c > +++ b/fs/cifs/transport.c > @@ -212,10 +212,24 @@ rqst_len(struct smb_rqst *rqst) > for (i = 0; i < rqst->rq_nvec; i++) > buflen += iov[i].iov_len; > > - /* add in the page array if there is one */ > + /* > + * Add in the page array if there is one. The caller needs to make > + * sure rq_offset and rq_tailsz are set correctly. If a buffer of > + * multiple pages ends at page boundary, rq_tailsz needs to be set to > + * PAGE_SIZE. > + */ > if (rqst->rq_npages) { > - buflen += rqst->rq_pagesz * (rqst->rq_npages - 1); > - buflen += rqst->rq_tailsz; > + if (rqst->rq_npages == 1) > + buflen += rqst->rq_tailsz; > + else { > + /* > + * If there is more than one page, calculate the > + * buffer length based on rq_offset and rq_tailsz > + */ > + buflen += rqst->rq_pagesz * (rqst->rq_npages - 1) - > + rqst->rq_offset; > + buflen += rqst->rq_tailsz; > + } Wouldn't it be simpler to keep the original code, but then just subtract the rq_offset? buflen += rqst->rq_pagesz * (rqst->rq_npages - 1); buflen += rqst->rq_tailsz; buflen -= rqst->rq_offset; It's kind of confusing as written. Also, what if it's just one page, but has a non-zero offset? Is that somehow not possible? My suggested code would take that into account, yours doesn't. Tom. > } > > return buflen; > -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
PiBTdWJqZWN0OiBSZTogW1BhdGNoIHYyIDA1LzE1XSBDSUZTOiBDYWxjdWxhdGUgdGhlIGNvcnJl Y3QgcmVxdWVzdCBsZW5ndGggYmFzZWQNCj4gb24gcGFnZSBvZmZzZXQgYW5kIHRhaWwgc2l6ZQ0K PiANCj4gT24gNS8zMC8yMDE4IDM6NDcgUE0sIExvbmcgTGkgd3JvdGU6DQo+ID4gRnJvbTogTG9u ZyBMaSA8bG9uZ2xpQG1pY3Jvc29mdC5jb20+DQo+ID4NCj4gPiBJdCdzIHBvc3NpYmxlIHRoYXQg dGhlIHBhZ2Ugb2Zmc2V0IGlzIG5vbi16ZXJvIGluIHRoZSBwYWdlcyBpbiBhDQo+ID4gcmVxdWVz dCwgY2hhbmdlIHRoZSBmdW5jdGlvbiB0byBjYWxjdWxhdGUgdGhlIGNvcnJlY3QgZGF0YSBidWZm ZXIgbGVuZ3RoLg0KPiA+DQo+ID4gU2lnbmVkLW9mZi1ieTogTG9uZyBMaSA8bG9uZ2xpQG1pY3Jv c29mdC5jb20+DQo+ID4gLS0tDQo+ID4gICBmcy9jaWZzL3RyYW5zcG9ydC5jIHwgMjAgKysrKysr KysrKysrKysrKystLS0NCj4gPiAgIDEgZmlsZSBjaGFuZ2VkLCAxNyBpbnNlcnRpb25zKCspLCAz IGRlbGV0aW9ucygtKQ0KPiA+DQo+ID4gZGlmZiAtLWdpdCBhL2ZzL2NpZnMvdHJhbnNwb3J0LmMg Yi9mcy9jaWZzL3RyYW5zcG9ydC5jIGluZGV4DQo+ID4gOTI3MjI2YS4uZDZiNTUyMyAxMDA2NDQN Cj4gPiAtLS0gYS9mcy9jaWZzL3RyYW5zcG9ydC5jDQo+ID4gKysrIGIvZnMvY2lmcy90cmFuc3Bv cnQuYw0KPiA+IEBAIC0yMTIsMTAgKzIxMiwyNCBAQCBycXN0X2xlbihzdHJ1Y3Qgc21iX3Jxc3Qg KnJxc3QpDQo+ID4gICAJZm9yIChpID0gMDsgaSA8IHJxc3QtPnJxX252ZWM7IGkrKykNCj4gPiAg IAkJYnVmbGVuICs9IGlvdltpXS5pb3ZfbGVuOw0KPiA+DQo+ID4gLQkvKiBhZGQgaW4gdGhlIHBh Z2UgYXJyYXkgaWYgdGhlcmUgaXMgb25lICovDQo+ID4gKwkvKg0KPiA+ICsJICogQWRkIGluIHRo ZSBwYWdlIGFycmF5IGlmIHRoZXJlIGlzIG9uZS4gVGhlIGNhbGxlciBuZWVkcyB0byBtYWtlDQo+ ID4gKwkgKiBzdXJlIHJxX29mZnNldCBhbmQgcnFfdGFpbHN6IGFyZSBzZXQgY29ycmVjdGx5LiBJ ZiBhIGJ1ZmZlciBvZg0KPiA+ICsJICogbXVsdGlwbGUgcGFnZXMgZW5kcyBhdCBwYWdlIGJvdW5k YXJ5LCBycV90YWlsc3ogbmVlZHMgdG8gYmUgc2V0IHRvDQo+ID4gKwkgKiBQQUdFX1NJWkUuDQo+ ID4gKwkgKi8NCj4gPiAgIAlpZiAocnFzdC0+cnFfbnBhZ2VzKSB7DQo+ID4gLQkJYnVmbGVuICs9 IHJxc3QtPnJxX3BhZ2VzeiAqIChycXN0LT5ycV9ucGFnZXMgLSAxKTsNCj4gPiAtCQlidWZsZW4g Kz0gcnFzdC0+cnFfdGFpbHN6Ow0KPiA+ICsJCWlmIChycXN0LT5ycV9ucGFnZXMgPT0gMSkNCj4g PiArCQkJYnVmbGVuICs9IHJxc3QtPnJxX3RhaWxzejsNCj4gPiArCQllbHNlIHsNCj4gPiArCQkJ LyoNCj4gPiArCQkJICogSWYgdGhlcmUgaXMgbW9yZSB0aGFuIG9uZSBwYWdlLCBjYWxjdWxhdGUg dGhlDQo+ID4gKwkJCSAqIGJ1ZmZlciBsZW5ndGggYmFzZWQgb24gcnFfb2Zmc2V0IGFuZCBycV90 YWlsc3oNCj4gPiArCQkJICovDQo+ID4gKwkJCWJ1ZmxlbiArPSBycXN0LT5ycV9wYWdlc3ogKiAo cnFzdC0+cnFfbnBhZ2VzIC0gMSkgLQ0KPiA+ICsJCQkJCXJxc3QtPnJxX29mZnNldDsNCj4gPiAr CQkJYnVmbGVuICs9IHJxc3QtPnJxX3RhaWxzejsNCj4gPiArCQl9DQo+IA0KPiBXb3VsZG4ndCBp dCBiZSBzaW1wbGVyIHRvIGtlZXAgdGhlIG9yaWdpbmFsIGNvZGUsIGJ1dCB0aGVuIGp1c3Qgc3Vi dHJhY3QgdGhlDQo+IHJxX29mZnNldD8NCj4gDQo+IAlidWZsZW4gKz0gcnFzdC0+cnFfcGFnZXN6 ICogKHJxc3QtPnJxX25wYWdlcyAtIDEpOw0KPiAJYnVmbGVuICs9IHJxc3QtPnJxX3RhaWxzejsN Cj4gCWJ1ZmxlbiAtPSBycXN0LT5ycV9vZmZzZXQ7DQo+IA0KPiBJdCdzIGtpbmQgb2YgY29uZnVz aW5nIGFzIHdyaXR0ZW4uIEFsc28sIHdoYXQgaWYgaXQncyBqdXN0IG9uZSBwYWdlLCBidXQgaGFz IGENCj4gbm9uLXplcm8gb2Zmc2V0PyBJcyB0aGF0IHNvbWVob3cgbm90IHBvc3NpYmxlPyBNeSBz dWdnZXN0ZWQgY29kZSB3b3VsZA0KPiB0YWtlIHRoYXQgaW50byBhY2NvdW50LCB5b3VycyBkb2Vz bid0Lg0KDQpJIHRoaW5rIEl0IGNhbiBiZSBjaGFuZ2VkIHRvIHRoaXM6DQoNCglidWZsZW4gKz0g cnFzdC0+cnFfcGFnZXN6ICogKHJxc3QtPnJxX25wYWdlcyAtIDEpOw0KIAlidWZsZW4gKz0gcnFz dC0+cnFfdGFpbHN6Ow0KCWlmIChycXN0LT5ycV9ucGFnZXMgPiAxKQ0KIAkJYnVmbGVuIC09IHJx c3QtPnJxX29mZnNldDsNCg0KVGhpcyBsb29rcyBjbGVhbmVyIHRoYW4gYmVmb3JlLg0KDQpXaGVu IHRoZXJlIGlzIG9ubHkgb25lIHBhZ2Ugd2l0aCBhIG5vbi16ZXJvIG9mZnNldCwgcnFfdGFpbHN6 IGlzIHRoZSBhY3R1YWwgZGF0YSBsZW5ndGguDQoNCj4gDQo+IFRvbS4NCj4gDQo+ID4gICAJfQ0K PiA+DQo+ID4gICAJcmV0dXJuIGJ1ZmxlbjsNCj4gPg0K -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c index 927226a..d6b5523 100644 --- a/fs/cifs/transport.c +++ b/fs/cifs/transport.c @@ -212,10 +212,24 @@ rqst_len(struct smb_rqst *rqst) for (i = 0; i < rqst->rq_nvec; i++) buflen += iov[i].iov_len; - /* add in the page array if there is one */ + /* + * Add in the page array if there is one. The caller needs to make + * sure rq_offset and rq_tailsz are set correctly. If a buffer of + * multiple pages ends at page boundary, rq_tailsz needs to be set to + * PAGE_SIZE. + */ if (rqst->rq_npages) { - buflen += rqst->rq_pagesz * (rqst->rq_npages - 1); - buflen += rqst->rq_tailsz; + if (rqst->rq_npages == 1) + buflen += rqst->rq_tailsz; + else { + /* + * If there is more than one page, calculate the + * buffer length based on rq_offset and rq_tailsz + */ + buflen += rqst->rq_pagesz * (rqst->rq_npages - 1) - + rqst->rq_offset; + buflen += rqst->rq_tailsz; + } } return buflen;